Bug 1366133 - Part 1: make nsISystemProxySettings::GetPACURI happen in another thread; r?bagder draft
authorLiang-Heng Chen <xeonchen@gmail.com>
Wed, 24 May 2017 14:36:53 +0800
changeset 590169 8455a7295f1229e20c9026f0139784afa36fba55
parent 590101 1ffe6203b5dbff3f9ea3021d59a16e97361fe540
child 632111 81bff7bfa767c9976763e4814624f6353e417f3a
push id62619
push userbmo:xeonchen@mozilla.com
push dateWed, 07 Jun 2017 08:47:07 +0000
reviewersbagder
bugs1366133
milestone55.0a1
Bug 1366133 - Part 1: make nsISystemProxySettings::GetPACURI happen in another thread; r?bagder MozReview-Commit-ID: FyiNM8KX0gk
netwerk/base/nsProtocolProxyService.cpp
netwerk/base/nsProtocolProxyService.h
toolkit/system/windowsproxy/nsWindowsSystemProxySettings.cpp
--- a/netwerk/base/nsProtocolProxyService.cpp
+++ b/netwerk/base/nsProtocolProxyService.cpp
@@ -17,31 +17,33 @@
 #include "nsIChannel.h"
 #include "nsICancelable.h"
 #include "nsIDNSService.h"
 #include "nsPIDNSService.h"
 #include "nsIScriptSecurityManager.h"
 #include "nsIPrefService.h"
 #include "nsIPrefBranch.h"
 #include "nsThreadUtils.h"
+#include "nsQueryObject.h"
 #include "nsSOCKSIOLayer.h"
 #include "nsString.h"
 #include "nsNetUtil.h"
 #include "nsNetCID.h"
 #include "plstr.h"
 #include "prnetdb.h"
 #include "nsPACMan.h"
 #include "nsProxyRelease.h"
 #include "mozilla/Mutex.h"
 #include "mozilla/CondVar.h"
 #include "nsISystemProxySettings.h"
 #include "nsINetworkLinkService.h"
 #include "nsIHttpChannelInternal.h"
 #include "mozilla/Logging.h"
 #include "mozilla/Tokenizer.h"
+#include "mozilla/Unused.h"
 
 //----------------------------------------------------------------------------
 
 namespace mozilla {
 namespace net {
 
   extern const char kProxyType_HTTP[];
   extern const char kProxyType_HTTPS[];
@@ -303,16 +305,84 @@ private:
     nsCOMPtr<nsIProtocolProxyService>  mXPComPPS;
     nsCOMPtr<nsIChannel>               mChannel;
     nsCOMPtr<nsIProtocolProxyCallback> mCallback;
     nsCOMPtr<nsIProxyInfo>             mProxyInfo;
 };
 
 NS_IMPL_ISUPPORTS(nsAsyncResolveRequest, nsICancelable, nsIRunnable)
 
+// Bug 1366133: make GetPACURI off-main-thread since it may hang on Windows platform
+class AsyncGetPACURIRequest final : public nsIRunnable
+{
+public:
+    NS_DECL_THREADSAFE_ISUPPORTS
+
+    using CallbackFunc = nsresult(nsProtocolProxyService::*)(bool, bool, nsresult, const nsACString&);
+
+    AsyncGetPACURIRequest(nsProtocolProxyService* aService,
+                          CallbackFunc aCallback,
+                          nsISystemProxySettings* aSystemProxySettings,
+                          bool aMainThreadOnly,
+                          bool aForceReload,
+                          bool aResetPACThread)
+        : mIsMainThreadOnly(aMainThreadOnly)
+        , mService(aService)
+        , mServiceHolder(do_QueryObject(aService))
+        , mCallback(aCallback)
+        , mSystemProxySettings(aSystemProxySettings)
+        , mForceReload(aForceReload)
+        , mResetPACThread(aResetPACThread)
+    {
+        MOZ_ASSERT(NS_IsMainThread());
+        Unused << mIsMainThreadOnly;
+    }
+
+    NS_IMETHOD Run() override
+    {
+        MOZ_ASSERT(NS_IsMainThread() == mIsMainThreadOnly);
+
+        nsCString pacUri;
+        nsresult rv = mSystemProxySettings->GetPACURI(pacUri);
+
+        nsCOMPtr<nsIRunnable> event =
+            NewNonOwningCancelableRunnableMethod<bool,
+                                                 bool,
+                                                 nsresult,
+                                                 nsCString>("AsyncGetPACURIRequestCallback",
+                                                             mService,
+                                                             mCallback,
+                                                             mForceReload,
+                                                             mResetPACThread,
+                                                             rv,
+                                                             pacUri);
+
+        return NS_DispatchToMainThread(event);
+    }
+
+private:
+    ~AsyncGetPACURIRequest()
+    {
+        MOZ_ASSERT(NS_IsMainThread() == mIsMainThreadOnly);
+        NS_ReleaseOnMainThread(mServiceHolder.forget());
+    }
+
+    bool mIsMainThreadOnly;
+
+    nsProtocolProxyService* mService; // ref-count is hold by mServiceHolder
+    nsCOMPtr<nsIProtocolProxyService2> mServiceHolder;
+    CallbackFunc mCallback;
+    nsCOMPtr<nsISystemProxySettings> mSystemProxySettings;
+
+    bool mForceReload;
+    bool mResetPACThread;
+};
+
+NS_IMPL_ISUPPORTS(AsyncGetPACURIRequest, nsIRunnable)
+
 //----------------------------------------------------------------------------
 
 #define IS_ASCII_SPACE(_c) ((_c) == ' ' || (_c) == '\t')
 
 //
 // apply mask to address (zeros out excluded bits).
 //
 // NOTE: we do the byte swapping here to minimize overall swapping.
@@ -457,16 +527,18 @@ nsProtocolProxyService::Init()
     nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
     if (obs) {
         // register for shutdown notification so we can clean ourselves up
         // properly.
         obs->AddObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID, false);
         obs->AddObserver(this, NS_NETWORK_LINK_TOPIC, false);
     }
 
+    NS_NewNamedThread("SysProxySetting", getter_AddRefs(mProxySettingThread));
+
     return NS_OK;
 }
 
 // ReloadNetworkPAC() checks if there's a non-networked PAC in use then avoids
 // to call ReloadPAC()
 nsresult
 nsProtocolProxyService::ReloadNetworkPAC()
 {
@@ -507,16 +579,64 @@ nsProtocolProxyService::ReloadNetworkPAC
         }
     } else if ((type == PROXYCONFIG_WPAD) || (type == PROXYCONFIG_SYSTEM)) {
         ReloadPAC();
     }
 
     return NS_OK;
 }
 
+nsresult
+nsProtocolProxyService::AsyncConfigureFromPAC(bool aForceReload,
+                                              bool aResetPACThread)
+{
+    MOZ_ASSERT(NS_IsMainThread());
+
+    if (NS_WARN_IF(!mProxySettingThread)) {
+        return NS_ERROR_NOT_INITIALIZED;
+    }
+
+    bool mainThreadOnly;
+    nsresult rv = mSystemProxySettings->GetMainThreadOnly(&mainThreadOnly);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+        return rv;
+    }
+
+    nsCOMPtr<nsIRunnable> req =
+        new AsyncGetPACURIRequest(this,
+                                  &nsProtocolProxyService::OnAsyncGetPACURI,
+                                  mSystemProxySettings,
+                                  mainThreadOnly,
+                                  aForceReload,
+                                  aResetPACThread);
+
+    if (mainThreadOnly) {
+        return req->Run();
+    }
+    return mProxySettingThread->Dispatch(req, nsIEventTarget::DISPATCH_NORMAL);
+}
+
+nsresult
+nsProtocolProxyService::OnAsyncGetPACURI(bool aForceReload,
+                                         bool aResetPACThread,
+                                         nsresult aResult,
+                                         const nsACString& aUri)
+{
+    MOZ_ASSERT(NS_IsMainThread());
+
+    if (aResetPACThread) {
+        ResetPACThread();
+    }
+
+    if (NS_SUCCEEDED(aResult) && !aUri.IsEmpty()) {
+        ConfigureFromPAC(PromiseFlatCString(aUri), aForceReload);
+    }
+
+    return NS_OK;
+}
 
 NS_IMETHODIMP
 nsProtocolProxyService::Observe(nsISupports     *aSubject,
                                 const char      *aTopic,
                                 const char16_t *aData)
 {
     if (strcmp(aTopic, NS_XPCOM_SHUTDOWN_OBSERVER_ID) == 0) {
         mIsShutdown = true;
@@ -528,16 +648,21 @@ nsProtocolProxyService::Observe(nsISuppo
             delete mFilters;
             mFilters = nullptr;
         }
         if (mPACMan) {
             mPACMan->Shutdown();
             mPACMan = nullptr;
         }
 
+        if (mProxySettingThread) {
+            mProxySettingThread->Shutdown();
+            mProxySettingThread = nullptr;
+        }
+
         nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
         if (obs) {
             obs->RemoveObserver(this, NS_NETWORK_LINK_TOPIC);
             obs->RemoveObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID);
         }
 
     } else if (strcmp(aTopic, NS_NETWORK_LINK_TOPIC) == 0) {
         nsCString converted = NS_ConvertUTF16toUTF8(aData);
@@ -679,17 +804,17 @@ nsProtocolProxyService::PrefsChanged(nsI
             // hosts's FQDN, stripping components until we hit a TLD.  Doing so
             // is dangerous in the face of an incomplete list of TLDs, and TLDs
             // get added over time.  We could consider doing only a single
             // substitution of the first component, if that proves to help
             // compatibility.
             tempString.AssignLiteral(WPAD_URL);
         } else if (mSystemProxySettings) {
             // Get System Proxy settings if available
-            mSystemProxySettings->GetPACURI(tempString);
+            AsyncConfigureFromPAC(false, false);
         }
         if (!tempString.IsEmpty())
             ConfigureFromPAC(tempString, false);
     }
 }
 
 bool
 nsProtocolProxyService::CanUseProxy(nsIURI *aURI, int32_t defaultPort)
@@ -1117,19 +1242,21 @@ nsProtocolProxyService::ReloadPAC()
         return NS_OK;
 
     nsXPIDLCString pacSpec;
     if (type == PROXYCONFIG_PAC)
         prefs->GetCharPref(PROXY_PREF("autoconfig_url"), getter_Copies(pacSpec));
     else if (type == PROXYCONFIG_WPAD)
         pacSpec.AssignLiteral(WPAD_URL);
     else if (type == PROXYCONFIG_SYSTEM) {
-        if (mSystemProxySettings)
-            mSystemProxySettings->GetPACURI(pacSpec);
-        ResetPACThread();
+        if (mSystemProxySettings) {
+            AsyncConfigureFromPAC(true, true);
+        } else {
+            ResetPACThread();
+        }
     }
 
     if (!pacSpec.IsEmpty())
         ConfigureFromPAC(pacSpec, true);
     return NS_OK;
 }
 
 // When sync interface is removed this can go away too
@@ -1810,16 +1937,19 @@ nsProtocolProxyService::Resolve_Internal
         *usePACThread = true;
         return NS_OK;
     }
 
     if (mSystemProxySettings && mProxyConfig == PROXYCONFIG_SYSTEM) {
         // If the system proxy setting implementation is not threadsafe (e.g
         // linux gconf), we'll do it inline here. Such implementations promise
         // not to block
+        // bug 1366133: this block uses GetPACURI & GetProxyForURI, which may
+        // hang on Windows platform. Fortunately, current implementation on
+        // Windows is not main thread only, so we are safe here.
 
         nsAutoCString PACURI;
         nsAutoCString pacString;
 
         if (NS_SUCCEEDED(mSystemProxySettings->GetPACURI(PACURI)) &&
             !PACURI.IsEmpty()) {
             // There is a PAC URI configured. If it is unchanged, then
             // just execute the PAC thread. If it is changed then load
--- a/netwerk/base/nsProtocolProxyService.h
+++ b/netwerk/base/nsProtocolProxyService.h
@@ -9,16 +9,17 @@
 #include "nsString.h"
 #include "nsCOMPtr.h"
 #include "nsAutoPtr.h"
 #include "nsTArray.h"
 #include "nsIProtocolProxyService2.h"
 #include "nsIProtocolProxyFilter.h"
 #include "nsIProxyInfo.h"
 #include "nsIObserver.h"
+#include "nsIThread.h"
 #include "nsDataHashtable.h"
 #include "nsHashKeys.h"
 #include "prio.h"
 #include "mozilla/Attributes.h"
 
 class nsIPrefBranch;
 class nsISystemProxySettings;
 
@@ -296,16 +297,22 @@ protected:
      */
     void MaybeDisableDNSPrefetch(nsIProxyInfo *aProxy);
 
 private:
     nsresult SetupPACThread();
     nsresult ResetPACThread();
     nsresult ReloadNetworkPAC();
 
+    nsresult AsyncConfigureFromPAC(bool aForceReload, bool aResetPACThread);
+    nsresult OnAsyncGetPACURI(bool aForceReload,
+                              bool aResetPACThread,
+                              nsresult aResult,
+                              const nsACString& aUri);
+
 public:
     // The Sun Forte compiler and others implement older versions of the
     // C++ standard's rules on access and nested classes.  These structs
     // need to be public in order to deal with those compilers.
 
     struct HostInfoIP {
         uint16_t   family;
         uint16_t   mask_len;
@@ -396,16 +403,17 @@ protected:
     int32_t                      mFailedProxyTimeout;
 
 private:
     nsresult AsyncResolveInternal(nsIChannel *channel, uint32_t flags,
                                   nsIProtocolProxyCallback *callback,
                                   nsICancelable **result,
                                   bool isSyncOK);
     bool                          mIsShutdown;
+    nsCOMPtr<nsIThread>           mProxySettingThread;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsProtocolProxyService, NS_PROTOCOL_PROXY_SERVICE_IMPL_CID)
 
 } // namespace net
 } // namespace mozilla
 
 #endif // !nsProtocolProxyService_h__
--- a/toolkit/system/windowsproxy/nsWindowsSystemProxySettings.cpp
+++ b/toolkit/system/windowsproxy/nsWindowsSystemProxySettings.cpp
@@ -11,16 +11,17 @@
 #include "mozilla/Attributes.h"
 #include "nsISystemProxySettings.h"
 #include "nsIServiceManager.h"
 #include "mozilla/ModuleUtils.h"
 #include "nsPrintfCString.h"
 #include "nsNetCID.h"
 #include "nsISupportsPrimitives.h"
 #include "nsIURI.h"
+#include "nsThreadUtils.h"
 #include "GeckoProfiler.h"
 #include "prnetdb.h"
 #include "ProxyUtils.h"
 
 class nsWindowsSystemProxySettings final : public nsISystemProxySettings
 {
 public:
     NS_DECL_THREADSAFE_ISUPPORTS
@@ -36,16 +37,19 @@ private:
     bool PatternMatch(const nsACString& aHost, const nsACString& aOverride);
 };
 
 NS_IMPL_ISUPPORTS(nsWindowsSystemProxySettings, nsISystemProxySettings)
 
 NS_IMETHODIMP
 nsWindowsSystemProxySettings::GetMainThreadOnly(bool *aMainThreadOnly)
 {
+  // bug 1366133: if you change this to main thread only, please handle
+  // nsProtocolProxyService::Resolve_Internal carefully to avoid hang on main
+  // thread.
   *aMainThreadOnly = false;
   return NS_OK;
 }
 
 
 nsresult
 nsWindowsSystemProxySettings::Init()
 {
@@ -64,16 +68,19 @@ static void SetProxyResultDirect(nsACStr
 {
     // For whatever reason, a proxy is not to be used.
     aResult.AssignASCII("DIRECT");
 }
 
 static nsresult ReadInternetOption(uint32_t aOption, uint32_t& aFlags,
                                    nsAString& aValue)
 {
+    // Bug 1366133: InternetGetConnectedStateExW() may cause hangs
+    MOZ_ASSERT(!NS_IsMainThread());
+
     DWORD connFlags = 0;
     WCHAR connName[RAS_MaxEntryName + 1];
     MOZ_SEH_TRY {
         InternetGetConnectedStateExW(&connFlags, connName,
                                      mozilla::ArrayLength(connName), 0);
     } MOZ_SEH_EXCEPT(EXCEPTION_EXECUTE_HANDLER) {
         return NS_ERROR_FAILURE;
     }