Synchronize events of CFNetworkExecuteProxyAutoConfigurationURL

net/proxy/proxy_resolver_mac.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/proxy/proxy_resolver_mac.h"
 
 #include <CoreFoundation/CoreFoundation.h>
 
+#include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/mac/foundation_util.h"
 #include "base/mac/scoped_cftyperef.h"
 #include "base/strings/string_util.h"
 #include "base/strings/sys_string_conversions.h"
+#include "base/synchronization/lock.h"
+#include "base/threading/thread_checker.h"
 #include "net/base/net_errors.h"
 #include "net/proxy/proxy_info.h"
 #include "net/proxy/proxy_resolver.h"
 #include "net/proxy/proxy_server.h"
 
 #if defined(OS_IOS)
 #include <CFNetwork/CFProxySupport.h>
 #else
 #include <CoreServices/CoreServices.h>
 #endif
 
 namespace net {
 
 namespace {
 
+// A lock shared by all ProxyResolverMac instances. It is used to synchronize
+// the events of multiple CFNetworkExecuteProxyAutoConfigurationURL run loop
+// sources. These events are:
+// 1. Adding the source to the run loop.
+// 2. Handling the source result.
+// 3. Removing the source from the run loop.
+static base::LazyInstance<base::Lock>::Leaky cfnetwork_pac_runloop_lock_ =

[mef, 2016/06/29 20:13:39]

nit: trailing underscore is used only for member variables, global should start
with g_.

[kapishnikov, 2016/06/29 22:41:11]

Done.

+    LAZY_INSTANCE_INITIALIZER;
+
+// Forward declaration of the callback function used by the
+// SynchronizedRunLoopObserver class.
+void RunLoopObserverCallBackFunc(CFRunLoopObserverRef observer,
+                                 CFRunLoopActivity activity,
+                                 void* info);
+
 // Utility function to map a CFProxyType to a ProxyServer::Scheme.
 // If the type is unknown, returns ProxyServer::SCHEME_INVALID.
 ProxyServer::Scheme GetProxyServerScheme(CFStringRef proxy_type) {
   if (CFEqual(proxy_type, kCFProxyTypeNone))
     return ProxyServer::SCHEME_DIRECT;
   if (CFEqual(proxy_type, kCFProxyTypeHTTP))
     return ProxyServer::SCHEME_HTTP;
   if (CFEqual(proxy_type, kCFProxyTypeHTTPS)) {
     // The "HTTPS" on the Mac side here means "proxy applies to https://" URLs;
     // the proxy itself is still expected to be an HTTP proxy.
@@ skipping 17 matching lines @@
   DCHECK(*result_ptr == NULL);
 
   if (error != NULL) {
     *result_ptr = CFRetain(error);
   } else {
     *result_ptr = CFRetain(proxies);
   }
   CFRunLoopStop(CFRunLoopGetCurrent());
 }
 
+#pragma mark - SynchronizedRunLoopObserver
+// A run loop observer that guarantees that no any two run loop sources
+// protected by the same lock will be fired concurrently in different threads.
+// The observer does not prevent the parallel execution of the sources but only
+// synchronizes the run loop events associated with the sources. In the context
+// of proxy resolver, the observer is used to synchronize the execution of the
+// callbacks function that handles the result of
+// CFNetworkExecuteProxyAutoConfigurationURL execution.
+class SynchronizedRunLoopObserver final {
+ public:
+  // Creates the instance of an observer that will synchronize the sources
+  // using a given |lock|.
+  SynchronizedRunLoopObserver(base::Lock& lock);
+  // Destructor.
+  ~SynchronizedRunLoopObserver();
+  // Adds the observer to the current run loop for a given run loop mode.
+  // This method should always be paired with |RemoveFromCurrentRunLoop|.
+  void AddToCurrentRunLoop(const CFStringRef mode);
+  // Removes the observer from the current run loop for a given run loop mode.
+  // This method should always be paired with |AddToCurrentRunLoop|.
+  void RemoveFromCurrentRunLoop(const CFStringRef mode);
+  // Callback function that is called when an observable run loop event occurs.
+  void RunLoopObserverCallBack(CFRunLoopObserverRef observer,
+                               CFRunLoopActivity activity);
+
+ private:
+  // Lock to use to synchronize the run loop sources.
+  base::Lock& lock_;
+  // Indicates whether the current observer holds the lock. It is used to
+  // avoid double locking and releasing.
+  bool lock_acquired_;
+  // The underlying CFRunLoopObserverRef structure wrapped by this instance.
+  base::ScopedCFTypeRef<CFRunLoopObserverRef> observer_;
+  // Validates that all methods of this class are executed on the same thread.
+  base::ThreadChecker thread_checker_;

[mef, 2016/06/29 20:13:39]

Add DISALLOW_COPY_AND_ASSIGN(SynchronizedRunLoopObserver);

[kapishnikov, 2016/06/29 22:41:11]

Done.

+};
+
+SynchronizedRunLoopObserver::SynchronizedRunLoopObserver(base::Lock& lock)
+    : lock_(lock), lock_acquired_(false) {
+  CFRunLoopObserverContext observer_context = {0, this, NULL, NULL, NULL};
+  observer_.reset(CFRunLoopObserverCreate(
+      kCFAllocatorDefault,
+      kCFRunLoopBeforeSources | kCFRunLoopBeforeWaiting | kCFRunLoopExit, true,
+      0, RunLoopObserverCallBackFunc, &observer_context));
+}
+
+SynchronizedRunLoopObserver::~SynchronizedRunLoopObserver() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(!lock_acquired_);
+}
+
+void SynchronizedRunLoopObserver::AddToCurrentRunLoop(const CFStringRef mode) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  CFRunLoopAddObserver(CFRunLoopGetCurrent(), observer_.get(), mode);
+}
+
+void SynchronizedRunLoopObserver::RemoveFromCurrentRunLoop(
+    const CFStringRef mode) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  CFRunLoopRemoveObserver(CFRunLoopGetCurrent(), observer_.get(), mode);
+}
+
+void SynchronizedRunLoopObserver::RunLoopObserverCallBack(
+    CFRunLoopObserverRef observer,
+    CFRunLoopActivity activity) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  // Acquire the lock when a source has been signaled and going to be fired.
+  // In the context of the proxy resolver that happens when the proxy for a
+  // given URL has been resolved and the callback function that handles the
+  // result is going to be fired.
+  // Release the lock when all source events have been handled.
+  switch (activity) {
+    case kCFRunLoopBeforeSources:
+      if (!lock_acquired_) {
+        lock_.Acquire();
+        lock_acquired_ = true;
+      }
+      break;
+    case kCFRunLoopBeforeWaiting:
+    case kCFRunLoopExit:
+      if (lock_acquired_) {
+        lock_acquired_ = false;
+        lock_.Release();
+      }
+      break;
+  }
+}
+
+void RunLoopObserverCallBackFunc(CFRunLoopObserverRef observer,
+                                 CFRunLoopActivity activity,
+                                 void* info) {
+  // Forward the call to the instance of SynchronizedRunLoopObserver
+  // that is associated with the current CF run loop observer.
+  SynchronizedRunLoopObserver* observerInstance =
+      (SynchronizedRunLoopObserver*)info;
+  observerInstance->RunLoopObserverCallBack(observer, activity);
+}
+
+#pragma mark - ProxyResolverMac
 class ProxyResolverMac : public ProxyResolver {
  public:
   explicit ProxyResolverMac(
       const scoped_refptr<ProxyResolverScriptData>& script_data);
   ~ProxyResolverMac() override;
 
   // ProxyResolver methods:
   int GetProxyForURL(const GURL& url,
                      ProxyInfo* results,
                      const CompletionCallback& callback,
@@ skipping 57 matching lines @@
   CFStreamClientContext context = { 0, &result, NULL, NULL, NULL };
   base::ScopedCFTypeRef<CFRunLoopSourceRef> runloop_source(
       CFNetworkExecuteProxyAutoConfigurationURL(
           pac_url_ref.get(), query_url_ref.get(), ResultCallback, &context));
   if (!runloop_source)
     return ERR_FAILED;
 
   const CFStringRef private_runloop_mode =
       CFSTR("org.chromium.ProxyResolverMac");
 
-  CFRunLoopAddSource(CFRunLoopGetCurrent(), runloop_source.get(),
-                     private_runloop_mode);
+  // Add the run loop observer to synchronize events of
+  // CFNetworkExecuteProxyAutoConfigurationURL sources. See the definition of
+  // |cfnetwork_pac_runloop_lock_|.
+  SynchronizedRunLoopObserver observer(cfnetwork_pac_runloop_lock_.Get());
+  observer.AddToCurrentRunLoop(private_runloop_mode);
+
+  // Make sure that no CFNetworkExecuteProxyAutoConfigurationURL sources
+  // are added to the run loop concurrently.
+  {
+    base::AutoLock lock(cfnetwork_pac_runloop_lock_.Get());
+    CFRunLoopAddSource(CFRunLoopGetCurrent(), runloop_source.get(),
+                       private_runloop_mode);
+  }
+
   CFRunLoopRunInMode(private_runloop_mode, DBL_MAX, false);
-  CFRunLoopSourceInvalidate(runloop_source.get());
+
+  // Make sure that no CFNetworkExecuteProxyAutoConfigurationURL sources
+  // are removed from the run loop concurrently.
+  {
+    base::AutoLock lock(cfnetwork_pac_runloop_lock_.Get());
+    CFRunLoopRemoveSource(CFRunLoopGetCurrent(), runloop_source.get(),
+                          private_runloop_mode);
+  }
+  observer.RemoveFromCurrentRunLoop(private_runloop_mode);
+
   DCHECK(result != NULL);
 
   if (CFGetTypeID(result) == CFErrorGetTypeID()) {
     // TODO(avi): do something better than this
     CFRelease(result);
     return ERR_FAILED;
   }
   base::ScopedCFTypeRef<CFArrayRef> proxy_array_ref(
       base::mac::CFCastStrict<CFArrayRef>(result));
   DCHECK(proxy_array_ref != NULL);
@@ skipping 64 matching lines @@
 int ProxyResolverFactoryMac::CreateProxyResolver(
     const scoped_refptr<ProxyResolverScriptData>& pac_script,
     std::unique_ptr<ProxyResolver>* resolver,
     const CompletionCallback& callback,
     std::unique_ptr<Request>* request) {
   resolver->reset(new ProxyResolverMac(pac_script));
   return OK;
 }
 
 }  // namespace net