From a064f414e34fbf772c6d76280d082e717375ed83 Mon Sep 17 00:00:00 2001 From: Eric Rahm Date: Mon, 16 Jul 2018 16:05:39 -0700 Subject: [PATCH] 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. --HG-- extra : rebase_source : 8897d3d8724f082ad33027635795512ccb4a17eb extra : source : 068bb4e7b8494d8ae82dfd1b1f22680234bf038c --- netwerk/base/ProxyAutoConfig.cpp | 4 +- netwerk/base/nsPACMan.cpp | 77 ++++++++++++++++++++----------- netwerk/base/nsPACMan.h | 15 ++++-- netwerk/test/gtest/TestPACMan.cpp | 4 +- 4 files changed, 67 insertions(+), 33 deletions(-) diff --git a/netwerk/base/ProxyAutoConfig.cpp b/netwerk/base/ProxyAutoConfig.cpp index f540871ec031..d2b32aeb5ae3 100644 --- a/netwerk/base/ProxyAutoConfig.cpp +++ b/netwerk/base/ProxyAutoConfig.cpp @@ -412,7 +412,7 @@ bool PACResolve(const nsCString &aHostName, NetAddr *aNetAddr, ProxyAutoConfig::ProxyAutoConfig() : mJSContext(nullptr) , mJSNeedsSetup(false) - , mShutdown(false) + , mShutdown(true) , mIncludePath(false) , mExtraHeapSize(0) { @@ -704,6 +704,8 @@ ProxyAutoConfig::Init(const nsCString &aPACURI, uint32_t aExtraHeapSize, nsIEventTarget *aEventTarget) { + mShutdown = false; // Shutdown needs to be called prior to destruction + mPACURI = aPACURI; mPACScript = sPacUtils; mPACScript.Append(aPACScript); diff --git a/netwerk/base/nsPACMan.cpp b/netwerk/base/nsPACMan.cpp index c081faeeadc7..6b0c1cd38eda 100644 --- a/netwerk/base/nsPACMan.cpp +++ b/netwerk/base/nsPACMan.cpp @@ -19,6 +19,8 @@ #include "nsISystemProxySettings.h" #include "nsNetUtil.h" #include "nsThreadUtils.h" +#include "mozilla/Result.h" +#include "mozilla/ResultExtensions.h" //----------------------------------------------------------------------------- @@ -266,12 +268,14 @@ public: , 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, @@ -294,7 +298,7 @@ public: { MOZ_ASSERT(!NS_IsMainThread(), "wrong thread"); if (mCancel) { - mPACMan->CancelPendingQ(mCancelStatus); + mPACMan->CancelPendingQ(mCancelStatus, mShutdown); mCancel = false; return NS_OK; } @@ -338,6 +342,7 @@ private: nsCString mSetupPACData; nsCString mSetupPACURI; bool mConfigureWPAD; + bool mShutdown; }; //----------------------------------------------------------------------------- @@ -446,14 +451,41 @@ nsPACMan::Shutdown() 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); - RefPtr runnable = new WaitForThreadShutdown(this); - Dispatch(runnable.forget()); + // 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 runnable = new WaitForThreadShutdown(this); + Dispatch(runnable.forget()); + } + + mShutdown = true; +} + +nsresult +nsPACMan::DispatchToPAC(already_AddRefed aEvent, bool aSync) +{ + MOZ_ASSERT(NS_IsMainThread(), "wrong thread"); + + nsCOMPtr 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(), aSync ? nsIEventTarget::DISPATCH_SYNC : + nsIEventTarget::DISPATCH_NORMAL); } nsresult @@ -482,7 +514,7 @@ nsPACMan::AsyncGetProxyForURI(nsIURI *uri, return NS_OK; } - return mPACThread->Dispatch(query, nsIEventTarget::DISPATCH_NORMAL); + return DispatchToPAC(query.forget()); } nsresult @@ -610,9 +642,7 @@ nsPACMan::StartLoading() RefPtr wpadConfigurer = new ExecutePACThreadAction(this); wpadConfigurer->ConfigureWPAD(); - if (mPACThread) { - mPACThread->Dispatch(wpadConfigurer, nsIEventTarget::DISPATCH_NORMAL); - } + DispatchToPAC(wpadConfigurer.forget()); } else { ContinueLoadingAfterPACUriKnown(); } @@ -716,23 +746,21 @@ nsPACMan::PostProcessPendingQ() MOZ_ASSERT(NS_IsMainThread(), "wrong thread"); RefPtr 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 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 query; @@ -742,7 +770,7 @@ nsPACMan::CancelPendingQ(nsresult status) query->Complete(status, EmptyCString()); } - if (mShutdown) + if (aShutdown) mPAC.Shutdown(); } @@ -869,8 +897,7 @@ nsPACMan::OnStreamComplete(nsIStreamLoader *loader, RefPtr 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")); @@ -946,12 +973,8 @@ 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 diff --git a/netwerk/base/nsPACMan.h b/netwerk/base/nsPACMan.h index 59afd75cf4ca..5a68592b9e83 100644 --- a/netwerk/base/nsPACMan.h +++ b/netwerk/base/nsPACMan.h @@ -176,7 +176,7 @@ public: // PAC thread operations only void ProcessPendingQ(); - void CancelPendingQ(nsresult); + void CancelPendingQ(nsresult, bool aShutdown); void SetWPADOverDHCPEnabled(bool aValue) { mWPADOverDHCPEnabled = aValue; } @@ -232,12 +232,21 @@ private: // 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. + * + * @param aEvent The event to disptach. + * @param aSync Whether or not this should be synchronous dispatch. + */ + nsresult DispatchToPAC(already_AddRefed aEvent, bool aSync = false); + ProxyAutoConfig mPAC; nsCOMPtr mPACThread; nsCOMPtr mSystemProxySettings; @@ -270,4 +279,4 @@ extern LazyLogModule gProxyLog; } // namespace net } // namespace mozilla -#endif // nsPACMan_h__ \ No newline at end of file +#endif // nsPACMan_h__ diff --git a/netwerk/test/gtest/TestPACMan.cpp b/netwerk/test/gtest/TestPACMan.cpp index d36a0a64a912..0fa6b1757398 100644 --- a/netwerk/test/gtest/TestPACMan.cpp +++ b/netwerk/test/gtest/TestPACMan.cpp @@ -201,7 +201,7 @@ class TestPACMan : public ::testing::Test { RefPtr action = new ProcessPendingEventsAction(); - mPACMan->mPACThread->Dispatch(action, nsIEventTarget::DISPATCH_SYNC); + mPACMan->DispatchToPAC(action.forget(), /*aSync =*/ true); } }; @@ -279,4 +279,4 @@ TEST_F(TestPACMan, WhenPACUriIsSetDirectlyItIsUsedRatherThanWPAD) { } } // namespace net -} // namespace mozilla \ No newline at end of file +} // namespace mozilla