Bug 1448034 - Part 2: Lazily create ProxyResolution thread. r=bagder
☠☠ backed out by 3e7a177e2e84 ☠ ☠
authorEric Rahm <erahm@mozilla.com>
Mon, 16 Jul 2018 16:05:39 -0700
changeset 426890 068bb4e7b8494d8ae82dfd1b1f22680234bf038c
parent 426889 eb542860b989d4f6ea1ffcb29ff843b857d78482
child 426891 bfb8b44ccc42b8f4e9073acf2b0763341e30efae
push id34289
push usertoros@mozilla.com
push dateTue, 17 Jul 2018 21:56:05 +0000
treeherdermozilla-central@afa310dc89be [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbagder
bugs1448034
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1448034 - Part 2: Lazily create ProxyResolution thread. r=bagder This delays the creation of the PAC thread until we need to dispatch a runnable to it.
netwerk/base/ProxyAutoConfig.cpp
netwerk/base/nsPACMan.cpp
netwerk/base/nsPACMan.h
--- a/netwerk/base/ProxyAutoConfig.cpp
+++ b/netwerk/base/ProxyAutoConfig.cpp
@@ -407,17 +407,17 @@ bool PACResolve(const nsCString &aHostNa
   }
 
   return GetRunning()->ResolveAddress(aHostName, aNetAddr, aTimeout);
 }
 
 ProxyAutoConfig::ProxyAutoConfig()
   : mJSContext(nullptr)
   , mJSNeedsSetup(false)
-  , mShutdown(false)
+  , mShutdown(true)
   , mIncludePath(false)
   , mExtraHeapSize(0)
 {
   MOZ_COUNT_CTOR(ProxyAutoConfig);
 }
 
 bool
 ProxyAutoConfig::ResolveAddress(const nsCString &aHostName,
@@ -699,16 +699,18 @@ ProxyAutoConfig::SetThreadLocalIndex(uin
 
 nsresult
 ProxyAutoConfig::Init(const nsCString &aPACURI,
                       const nsCString &aPACScript,
                       bool aIncludePath,
                       uint32_t aExtraHeapSize,
                       nsIEventTarget *aEventTarget)
 {
+  mShutdown = false; // Shutdown needs to be called prior to destruction
+
   mPACURI = aPACURI;
   mPACScript = sPacUtils;
   mPACScript.Append(aPACScript);
   mIncludePath = aIncludePath;
   mExtraHeapSize = aExtraHeapSize;
   mMainThreadEventTarget = aEventTarget;
 
   if (!GetRunning())
--- a/netwerk/base/nsPACMan.cpp
+++ b/netwerk/base/nsPACMan.cpp
@@ -14,16 +14,18 @@
 #include "nsIHttpChannel.h"
 #include "nsIPrefService.h"
 #include "nsIPrefBranch.h"
 #include "nsIPromptFactory.h"
 #include "nsIProtocolProxyService.h"
 #include "nsISystemProxySettings.h"
 #include "nsNetUtil.h"
 #include "nsThreadUtils.h"
+#include "mozilla/Result.h"
+#include "mozilla/ResultExtensions.h"
 
 //-----------------------------------------------------------------------------
 
 namespace mozilla {
 namespace net {
 
 LazyLogModule gProxyLog("proxy");
 
@@ -261,22 +263,24 @@ public:
   explicit ExecutePACThreadAction(nsPACMan* aPACMan)
     : Runnable("net::ExecutePACThreadAction")
     , mPACMan(aPACMan)
     , mCancel(false)
     , mCancelStatus(NS_OK)
     , mSetupPAC(false)
     , mExtraHeapSize(0)
     , mConfigureWPAD(false)
+    , mShutdown(false)
   { }
 
-  void CancelQueue (nsresult status)
+  void CancelQueue (nsresult status, bool aShutdown)
   {
     mCancel = true;
     mCancelStatus = status;
+    mShutdown = aShutdown;
   }
 
   void SetupPAC (const char *text,
                  uint32_t datalen,
                  const nsACString &pacURI,
                  uint32_t extraHeapSize)
   {
     mSetupPAC = true;
@@ -289,17 +293,17 @@ public:
   {
     mConfigureWPAD = true;
   }
 
   NS_IMETHOD Run() override
   {
     MOZ_ASSERT(!NS_IsMainThread(), "wrong thread");
     if (mCancel) {
-      mPACMan->CancelPendingQ(mCancelStatus);
+      mPACMan->CancelPendingQ(mCancelStatus, mShutdown);
       mCancel = false;
       return NS_OK;
     }
 
     if (mSetupPAC) {
       mSetupPAC = false;
 
       nsCOMPtr<nsIEventTarget> target = mPACMan->GetNeckoTarget();
@@ -333,16 +337,17 @@ private:
   bool      mCancel;
   nsresult  mCancelStatus;
 
   bool                 mSetupPAC;
   uint32_t             mExtraHeapSize;
   nsCString            mSetupPACData;
   nsCString            mSetupPACURI;
   bool                 mConfigureWPAD;
+  bool                 mShutdown;
 };
 
 //-----------------------------------------------------------------------------
 
 PendingPACQuery::PendingPACQuery(nsPACMan* pacMan,
                                  nsIURI* uri,
                                  nsPACManCallback* callback,
                                  bool mainThreadResponse)
@@ -441,24 +446,50 @@ nsPACMan::~nsPACMan()
 
 void
 nsPACMan::Shutdown()
 {
   MOZ_ASSERT(NS_IsMainThread(), "pacman must be shutdown on main thread");
   if (mShutdown) {
     return;
   }
-  mShutdown = true;
+
   CancelExistingLoad();
 
-  MOZ_ASSERT(mPACThread, "mPAC requires mPACThread to shutdown");
-  PostCancelPendingQ(NS_ERROR_ABORT);
+  if (mPACThread) {
+    PostCancelPendingQ(NS_ERROR_ABORT, /*aShutdown =*/ true);
+
+    // Shutdown is initiated from an observer. We don't want to block the
+    // observer service on thread shutdown so we post a shutdown runnable that
+    // will run after we return instead.
+    RefPtr<WaitForThreadShutdown> runnable = new WaitForThreadShutdown(this);
+    Dispatch(runnable.forget());
+  }
+
+  mShutdown = true;
+}
 
-  RefPtr<WaitForThreadShutdown> runnable = new WaitForThreadShutdown(this);
-  Dispatch(runnable.forget());
+nsresult
+nsPACMan::DispatchToPAC(already_AddRefed<nsIRunnable> aEvent)
+{
+  MOZ_ASSERT(NS_IsMainThread(), "wrong thread");
+
+  nsCOMPtr<nsIRunnable> e(aEvent);
+
+  if (mShutdown) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+
+  // Lazily create the PAC thread. This method is main-thread only so we don't
+  // have to worry about threading issues here.
+  if (!mPACThread) {
+    MOZ_TRY(NS_NewNamedThread("ProxyResolution", getter_AddRefs(mPACThread)));
+  }
+
+  return mPACThread->Dispatch(e.forget(), nsIEventTarget::DISPATCH_NORMAL);
 }
 
 nsresult
 nsPACMan::AsyncGetProxyForURI(nsIURI *uri,
                               nsPACManCallback *callback,
                               bool mainThreadResponse)
 {
   MOZ_ASSERT(NS_IsMainThread(), "wrong thread");
@@ -477,17 +508,17 @@ nsPACMan::AsyncGetProxyForURI(nsIURI *ur
     new PendingPACQuery(this, uri, callback, mainThreadResponse);
 
   if (IsPACURI(uri)) {
     // deal with this directly instead of queueing it
     query->Complete(NS_OK, EmptyCString());
     return NS_OK;
   }
 
-  return mPACThread->Dispatch(query, nsIEventTarget::DISPATCH_NORMAL);
+  return DispatchToPAC(query.forget());
 }
 
 nsresult
 nsPACMan::PostQuery(PendingPACQuery *query)
 {
   MOZ_ASSERT(!NS_IsMainThread(), "wrong thread");
 
   if (mShutdown) {
@@ -605,19 +636,17 @@ nsPACMan::StartLoading()
     return;
   }
 
   if (mAutoDetect) {
     GetNetworkProxyTypeFromPref(&mProxyConfigType);
     RefPtr<ExecutePACThreadAction> wpadConfigurer =
       new ExecutePACThreadAction(this);
     wpadConfigurer->ConfigureWPAD();
-    if (mPACThread) {
-      mPACThread->Dispatch(wpadConfigurer, nsIEventTarget::DISPATCH_NORMAL);
-    }
+    DispatchToPAC(wpadConfigurer.forget());
   } else {
     ContinueLoadingAfterPACUriKnown();
   }
 }
 
 void
 nsPACMan::ContinueLoadingAfterPACUriKnown()
 {
@@ -711,43 +740,41 @@ nsPACMan::CancelExistingLoad()
 }
 
 void
 nsPACMan::PostProcessPendingQ()
 {
   MOZ_ASSERT(NS_IsMainThread(), "wrong thread");
   RefPtr<ExecutePACThreadAction> pending =
     new ExecutePACThreadAction(this);
-  if (mPACThread)
-    mPACThread->Dispatch(pending, nsIEventTarget::DISPATCH_NORMAL);
+  DispatchToPAC(pending.forget());
 }
 
 void
-nsPACMan::PostCancelPendingQ(nsresult status)
+nsPACMan::PostCancelPendingQ(nsresult status, bool aShutdown)
 {
   MOZ_ASSERT(NS_IsMainThread(), "wrong thread");
   RefPtr<ExecutePACThreadAction> pending =
     new ExecutePACThreadAction(this);
-  pending->CancelQueue(status);
-  if (mPACThread)
-    mPACThread->Dispatch(pending, nsIEventTarget::DISPATCH_NORMAL);
+  pending->CancelQueue(status, aShutdown);
+  DispatchToPAC(pending.forget());
 }
 
 void
-nsPACMan::CancelPendingQ(nsresult status)
+nsPACMan::CancelPendingQ(nsresult status, bool aShutdown)
 {
   MOZ_ASSERT(!NS_IsMainThread(), "wrong thread");
   RefPtr<PendingPACQuery> query;
 
   while (!mPendingQ.isEmpty()) {
     query = dont_AddRef(mPendingQ.popLast());
     query->Complete(status, EmptyCString());
   }
 
-  if (mShutdown)
+  if (aShutdown)
     mPAC.Shutdown();
 }
 
 void
 nsPACMan::ProcessPendingQ()
 {
   MOZ_ASSERT(!NS_IsMainThread(), "wrong thread");
   while (ProcessPending());
@@ -864,18 +891,17 @@ nsPACMan::OnStreamComplete(nsIStreamLoad
     // we have succeeded in loading the pac file using a bunch of interfaces that
     // are main thread only, unfortunately we have to initialize the instance of
     // the PAC evaluator (NS_PROXYAUTOCONFIG_CONTRACTID) on the pac thread, because
     // that is where it will be used.
 
     RefPtr<ExecutePACThreadAction> pending =
       new ExecutePACThreadAction(this);
     pending->SetupPAC(text, dataLen, pacURI, GetExtraJSContextHeapSize());
-    if (mPACThread)
-      mPACThread->Dispatch(pending, nsIEventTarget::DISPATCH_NORMAL);
+    DispatchToPAC(pending.forget());
 
     LOG(("OnStreamComplete: process the PAC contents\n"));
 
     // Even if the PAC file could not be parsed, we did succeed in loading the
     // data for it.
     mLoadFailureCount = 0;
   } else {
     // We were unable to load the PAC file (presumably because of a network
@@ -941,17 +967,13 @@ nsPACMan::AsyncOnChannelRedirect(nsIChan
   return NS_OK;
 }
 
 nsresult
 nsPACMan::Init(nsISystemProxySettings *systemProxySettings)
 {
   mSystemProxySettings = systemProxySettings;
   mDHCPClient = do_GetService(NS_DHCPCLIENT_CONTRACTID);
-
-  nsresult rv =
-    NS_NewNamedThread("ProxyResolution", getter_AddRefs(mPACThread));
-
-  return rv;
+  return NS_OK;
 }
 
 } // namespace net
-} // namespace mozilla
\ No newline at end of file
+} // namespace mozilla
--- a/netwerk/base/nsPACMan.h
+++ b/netwerk/base/nsPACMan.h
@@ -171,17 +171,17 @@ public:
     return mAutoDetect;
   }
 
   nsresult Init(nsISystemProxySettings *);
   static nsPACMan *sInstance;
 
   // PAC thread operations only
   void ProcessPendingQ();
-  void CancelPendingQ(nsresult);
+  void CancelPendingQ(nsresult, bool aShutdown);
 
   void SetWPADOverDHCPEnabled(bool aValue) { mWPADOverDHCPEnabled = aValue; }
 
 private:
   NS_DECL_NSISTREAMLOADEROBSERVER
   NS_DECL_NSIINTERFACEREQUESTOR
   NS_DECL_NSICHANNELEVENTSINK
 
@@ -227,22 +227,28 @@ private:
   nsresult PostQuery(PendingPACQuery *query);
 
   // Having found the PAC URI on the PAC thread, copy it to a string which
   // can be altered on the main thread.
   void AssignPACURISpec(const nsACString &aSpec);
 
   // PAC thread operations only
   void PostProcessPendingQ();
-  void PostCancelPendingQ(nsresult);
+  void PostCancelPendingQ(nsresult, bool aShutdown = false);
   bool ProcessPending();
   nsresult GetPACFromDHCP(nsACString &aSpec);
   nsresult ConfigureWPAD(nsACString &aSpec);
 
 private:
+  /**
+   * Dispatches a runnable to the PAC processing thread. Handles lazy
+   * instantiation of the thread.
+   */
+  nsresult DispatchToPAC(already_AddRefed<nsIRunnable> aEvent);
+
   ProxyAutoConfig mPAC;
   nsCOMPtr<nsIThread>           mPACThread;
   nsCOMPtr<nsISystemProxySettings> mSystemProxySettings;
   nsCOMPtr<nsIDHCPClient> mDHCPClient;
 
   LinkedList<PendingPACQuery> mPendingQ; /* pac thread only */
 
   // These specs are not nsIURI so that they can be used off the main thread.
@@ -265,9 +271,9 @@ private:
   int32_t                      mProxyConfigType;
 };
 
 extern LazyLogModule gProxyLog;
 
 } // namespace net
 } // namespace mozilla
 
-#endif  // nsPACMan_h__
\ No newline at end of file
+#endif  // nsPACMan_h__