Bug 1366133 - Part 1: make nsISystemProxySettings::GetPACURI happen in another thread; r=bagder

MozReview-Commit-ID: FyiNM8KX0gk

--HG--
extra : rebase_source : 60bc91300a08f8df76f439d73739c34aa32b2f92

netwerk/base/nsProtocolProxyService.cpp

@@ -22,6 +22,7 @@
 #include "nsIPrefService.h"
 #include "nsIPrefBranch.h"
 #include "nsThreadUtils.h"
+#include "nsQueryObject.h"
 #include "nsSOCKSIOLayer.h"
 #include "nsString.h"
 #include "nsNetUtil.h"
@@ -37,6 +38,7 @@
 #include "nsIHttpChannelInternal.h"
 #include "mozilla/Logging.h"
 #include "mozilla/Tokenizer.h"
+#include "mozilla/Unused.h"
 
 //----------------------------------------------------------------------------
 
@@ -308,6 +310,74 @@ private:
 
 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')
@@ -462,6 +532,8 @@ nsProtocolProxyService::Init()
         obs->AddObserver(this, NS_NETWORK_LINK_TOPIC, false);
     }
 
+    NS_NewNamedThread("SysProxySetting", getter_AddRefs(mProxySettingThread));
+
     return NS_OK;
 }
 
@@ -512,6 +584,54 @@ nsProtocolProxyService::ReloadNetworkPAC()
     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,
@@ -533,6 +653,11 @@ nsProtocolProxyService::Observe(nsISupports     *aSubject,
             mPACMan = nullptr;
         }
 
+        if (mProxySettingThread) {
+            mProxySettingThread->Shutdown();
+            mProxySettingThread = nullptr;
+        }
+
         nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
         if (obs) {
             obs->RemoveObserver(this, NS_NETWORK_LINK_TOPIC);
@@ -684,7 +809,7 @@ nsProtocolProxyService::PrefsChanged(nsIPrefBranch *prefBranch,
             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);
@@ -1122,9 +1247,11 @@ nsProtocolProxyService::ReloadPAC()
     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())
@@ -1815,6 +1942,9 @@ nsProtocolProxyService::Resolve_Internal(nsIChannel *channel,
         // 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;

netwerk/base/nsProtocolProxyService.h

@@ -14,6 +14,7 @@
 #include "nsIProtocolProxyFilter.h"
 #include "nsIProxyInfo.h"
 #include "nsIObserver.h"
+#include "nsIThread.h"
 #include "nsDataHashtable.h"
 #include "nsHashKeys.h"
 #include "prio.h"
@@ -301,6 +302,12 @@ private:
     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
@@ -401,6 +408,7 @@ private:
                                   nsICancelable **result,
                                   bool isSyncOK);
     bool                          mIsShutdown;
+    nsCOMPtr<nsIThread>           mProxySettingThread;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsProtocolProxyService, NS_PROTOCOL_PROXY_SERVICE_IMPL_CID)

toolkit/system/windowsproxy/nsWindowsSystemProxySettings.cpp

@@ -16,6 +16,7 @@
 #include "nsNetCID.h"
 #include "nsISupportsPrimitives.h"
 #include "nsIURI.h"
+#include "nsThreadUtils.h"
 #include "GeckoProfiler.h"
 #include "prnetdb.h"
 #include "ProxyUtils.h"
@@ -41,6 +42,9 @@ 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;
 }
@@ -69,6 +73,9 @@ static void SetProxyResultDirect(nsACString& aResult)
 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 {