diff --git a/netwerk/base/ProxyAutoConfig.cpp b/netwerk/base/ProxyAutoConfig.cpp index acb61b7334d0..299e98cba05d 100644 --- a/netwerk/base/ProxyAutoConfig.cpp +++ b/netwerk/base/ProxyAutoConfig.cpp @@ -257,26 +257,21 @@ bool ProxyAutoConfig::ResolveAddress(const nsCString& aHostName, } } - mWaitingForDNSResolve = true; // Spin the event loop of the pac thread until lookup is complete. // nsPACman is responsible for keeping a queue and only allowing // one PAC execution at a time even when it is called re-entrantly. SpinEventLoopUntil("ProxyAutoConfig::ResolveAddress"_ns, [&, helper, this]() { if (!helper->mRequest) { - mWaitingForDNSResolve = false; return true; } if (this->mShutdown) { NS_WARNING("mShutdown set with PAC request not cancelled"); MOZ_ASSERT(NS_FAILED(helper->mStatus)); - mWaitingForDNSResolve = false; return true; } return false; }); - MaybeInvokeDNSResolveCallbacks(); - if (NS_FAILED(helper->mStatus)) { return false; } @@ -612,31 +607,9 @@ nsresult ProxyAutoConfig::SetupJS() { mConcatenatedPACData.Truncate(); mPACURI.Truncate(); - MaybeInvokeDNSResolveCallbacks(); return NS_OK; } -void ProxyAutoConfig::MaybeInvokeDNSResolveCallbacks() { - if (mDNSResolveCallbacks.IsEmpty()) { - return; - } - - // This function could be called in the middle of SetupJS(). If this is the - // case, we should wait until mJSContext is ready to use. Otherwise, - // GetProxyForURI() would fail. - if (!mJSContext || !mJSContext->IsOK()) { - return; - } - - NS_DispatchToCurrentThread( - NS_NewRunnableFunction("InvokeDNSResolveCallback", - [callbacks{std::move(mDNSResolveCallbacks)}]() { - for (auto& callback : callbacks) { - callback(); - } - })); -} - void ProxyAutoConfig::GetProxyForURIWithCallback( const nsCString& aTestURI, const nsCString& aTestHost, std::function&& @@ -739,7 +712,6 @@ void ProxyAutoConfig::Shutdown() { mShutdown = true; delete mJSContext; mJSContext = nullptr; - mDNSResolveCallbacks.Clear(); } bool ProxyAutoConfig::SrcAddress(const NetAddr* remoteAddress, @@ -930,7 +902,10 @@ nsresult RemoteProxyAutoConfig::ConfigurePAC(const nsCString& aPACURI, void RemoteProxyAutoConfig::Shutdown() { mProxyAutoConfigParent->Close(); } -void RemoteProxyAutoConfig::GC() { Unused << mProxyAutoConfigParent->SendGC(); } +void RemoteProxyAutoConfig::GC() { + // Do nothing. GC would be performed when there is not pending query in socket + // process. +} void RemoteProxyAutoConfig::GetProxyForURIWithCallback( const nsCString& aTestURI, const nsCString& aTestHost, diff --git a/netwerk/base/ProxyAutoConfig.h b/netwerk/base/ProxyAutoConfig.h index a0f79bcd83ea..021a8736800b 100644 --- a/netwerk/base/ProxyAutoConfig.h +++ b/netwerk/base/ProxyAutoConfig.h @@ -104,11 +104,6 @@ class ProxyAutoConfig : public ProxyAutoConfigBase { std::function&& aCallback) override; - bool WaitingForDNSResolve() const { return mWaitingForDNSResolve; } - void RegisterDNSResolveCallback(std::function&& aCallback) { - mDNSResolveCallbacks.AppendElement(std::move(aCallback)); - } - private: // allow 665ms for myipaddress dns queries. That's 95th percentile. const static unsigned int kTimeout = 665; @@ -119,7 +114,6 @@ class ProxyAutoConfig : public ProxyAutoConfigBase { bool SrcAddress(const NetAddr* remoteAddress, nsCString& localAddress); bool MyIPAddressTryHost(const nsCString& hostName, unsigned int timeout, const JS::CallArgs& aArgs, bool* aResult); - void MaybeInvokeDNSResolveCallbacks(); JSContextWrapper* mJSContext{nullptr}; bool mJSNeedsSetup{false}; @@ -127,12 +121,10 @@ class ProxyAutoConfig : public ProxyAutoConfigBase { nsCString mConcatenatedPACData; nsCString mPACURI; bool mIncludePath{false}; - bool mWaitingForDNSResolve{false}; uint32_t mExtraHeapSize{0}; nsCString mRunningHost; nsCOMPtr mTimer; nsCOMPtr mMainThreadEventTarget; - nsTArray> mDNSResolveCallbacks; }; class RemoteProxyAutoConfig : public ProxyAutoConfigBase { diff --git a/netwerk/ipc/PProxyAutoConfig.ipdl b/netwerk/ipc/PProxyAutoConfig.ipdl index aa10da0634ac..105839743346 100644 --- a/netwerk/ipc/PProxyAutoConfig.ipdl +++ b/netwerk/ipc/PProxyAutoConfig.ipdl @@ -14,8 +14,6 @@ child: bool aIncludePath, uint32_t aExtraHeapSize); async GetProxyForURI(nsCString aTestURI, nsCString aTestHost) returns (nsresult aStatus, nsCString aResult); - async GC(); - }; } // namespace net diff --git a/netwerk/ipc/ProxyAutoConfigChild.cpp b/netwerk/ipc/ProxyAutoConfigChild.cpp index db0f1821bdca..b375216b1f90 100644 --- a/netwerk/ipc/ProxyAutoConfigChild.cpp +++ b/netwerk/ipc/ProxyAutoConfigChild.cpp @@ -19,7 +19,7 @@ static bool sThreadLocalSetup = false; static uint32_t sThreadLocalIndex = 0xdeadbeef; StaticRefPtr ProxyAutoConfigChild::sPACThread; bool ProxyAutoConfigChild::sShutdownObserverRegistered = false; -Atomic ProxyAutoConfigChild::sLiveActorCount(0); +static StaticRefPtr sActor; namespace { @@ -45,6 +45,27 @@ ShutdownObserver::Observe(nsISupports* aSubject, const char* aTopic, } // namespace +// static +void ProxyAutoConfigChild::BindProxyAutoConfigChild( + RefPtr&& aActor, + Endpoint&& aEndpoint) { + // We only allow one ProxyAutoConfigChild at a time, so we need to + // wait until the old one to be destroyed. + if (sActor) { + NS_DispatchToCurrentThread(NS_NewRunnableFunction( + "BindProxyAutoConfigChild", + [actor = std::move(aActor), endpoint = std::move(aEndpoint)]() mutable { + ProxyAutoConfigChild::BindProxyAutoConfigChild(std::move(actor), + std::move(endpoint)); + })); + return; + } + + if (aEndpoint.Bind(aActor)) { + sActor = aActor; + } +} + // static bool ProxyAutoConfigChild::Create(Endpoint&& aEndpoint) { if (!sPACThread && !CreatePACThread()) { @@ -66,22 +87,14 @@ bool ProxyAutoConfigChild::Create(Endpoint&& aEndpoint) { } RefPtr actor = new ProxyAutoConfigChild(); - if (NS_FAILED(sPACThread->Dispatch( - NS_NewRunnableFunction("ProxyAutoConfigChild::ProxyAutoConfigChild", - [actor = std::move(actor), - endpoint = std::move(aEndpoint)]() mutable { - MOZ_ASSERT(endpoint.IsValid()); - - // Transfer ownership to PAC thread. If - // Bind() fails then we will release this - // reference in Destroy. - ProxyAutoConfigChild* actorTmp; - actor.forget(&actorTmp); - - if (!endpoint.Bind(actorTmp)) { - actorTmp->Destroy(); - } - })))) { + if (NS_FAILED(sPACThread->Dispatch(NS_NewRunnableFunction( + "ProxyAutoConfigChild::ProxyAutoConfigChild", + [actor = std::move(actor), + endpoint = std::move(aEndpoint)]() mutable { + MOZ_ASSERT(endpoint.IsValid()); + ProxyAutoConfigChild::BindProxyAutoConfigChild(std::move(actor), + std::move(endpoint)); + })))) { NS_WARNING("Failed to dispatch runnable!"); return false; } @@ -115,7 +128,7 @@ void ProxyAutoConfigChild::ShutdownPACThread() { if (sPACThread) { // Wait until all actos are released. SpinEventLoopUntil("ProxyAutoConfigChild::ShutdownPACThread"_ns, - [&]() { return !sLiveActorCount; }); + [&]() { return !sActor; }); nsCOMPtr thread = sPACThread.get(); sPACThread = nullptr; @@ -131,52 +144,74 @@ ProxyAutoConfigChild::ProxyAutoConfigChild() } mPAC->SetThreadLocalIndex(sThreadLocalIndex); - sLiveActorCount++; } -ProxyAutoConfigChild::~ProxyAutoConfigChild() { - MOZ_ASSERT(NS_IsMainThread()); - sLiveActorCount--; -} +ProxyAutoConfigChild::~ProxyAutoConfigChild() = default; mozilla::ipc::IPCResult ProxyAutoConfigChild::RecvConfigurePAC( const nsCString& aPACURI, const nsCString& aPACScriptData, const bool& aIncludePath, const uint32_t& aExtraHeapSize) { mPAC->ConfigurePAC(aPACURI, aPACScriptData, aIncludePath, aExtraHeapSize, GetMainThreadSerialEventTarget()); + mPACLoaded = true; + NS_DispatchToCurrentThread( + NewRunnableMethod("ProxyAutoConfigChild::ProcessPendingQ", this, + &ProxyAutoConfigChild::ProcessPendingQ)); return IPC_OK(); } +void ProxyAutoConfigChild::PendingQuery::Resolve(nsresult aStatus, + const nsCString& aResult) { + mResolver(Tuple(aStatus, aResult)); +} + mozilla::ipc::IPCResult ProxyAutoConfigChild::RecvGetProxyForURI( const nsCString& aTestURI, const nsCString& aTestHost, GetProxyForURIResolver&& aResolver) { - RefPtr self = this; - auto callResolver = [self, testURI(aTestURI), testHost(aTestHost), - resolver{std::move(aResolver)}]() { - if (!self->CanSend()) { - return; - } - - nsCString result; - nsresult rv = self->mPAC->GetProxyForURI(testURI, testHost, result); - resolver(Tuple(rv, result)); - }; - if (mPAC->WaitingForDNSResolve()) { - mPAC->RegisterDNSResolveCallback( - [resolverCallback{std::move(callResolver)}]() { resolverCallback(); }); - return IPC_OK(); - } - - callResolver(); + mPendingQ.insertBack( + new PendingQuery(aTestURI, aTestHost, std::move(aResolver))); + ProcessPendingQ(); return IPC_OK(); } -mozilla::ipc::IPCResult ProxyAutoConfigChild::RecvGC() { - mPAC->GC(); - return IPC_OK(); +void ProxyAutoConfigChild::ProcessPendingQ() { + while (ProcessPending()) { + ; + } + + if (mShutdown) { + mPAC->Shutdown(); + } else { + // do GC while the thread has nothing pending + mPAC->GC(); + } +} + +bool ProxyAutoConfigChild::ProcessPending() { + if (mPendingQ.isEmpty()) { + return false; + } + + if (mInProgress || !mPACLoaded) { + return false; + } + + if (mShutdown) { + return true; + } + + mInProgress = true; + RefPtr query = mPendingQ.popFirst(); + nsCString result; + nsresult rv = mPAC->GetProxyForURI(query->URI(), query->Host(), result); + query->Resolve(rv, result); + mInProgress = false; + return true; } void ProxyAutoConfigChild::ActorDestroy(ActorDestroyReason aWhy) { + mPendingQ.clear(); + mShutdown = true; mPAC->Shutdown(); // To avoid racing with the main thread, we need to dispatch @@ -185,18 +220,6 @@ void ProxyAutoConfigChild::ActorDestroy(ActorDestroyReason aWhy) { "ProxyAutoConfigChild::Destroy", this, &ProxyAutoConfigChild::Destroy))); } -void ProxyAutoConfigChild::Destroy() { - // May be called on any thread! - MOZ_ALWAYS_SUCCEEDS(NS_DispatchToMainThread(NewNonOwningRunnableMethod( - "ProxyAutoConfigChild::MainThreadActorDestroy", this, - &ProxyAutoConfigChild::MainThreadActorDestroy))); -} - -void ProxyAutoConfigChild::MainThreadActorDestroy() { - MOZ_ASSERT(NS_IsMainThread()); - - // This may be the last reference! - Release(); -} +void ProxyAutoConfigChild::Destroy() { sActor = nullptr; } } // namespace mozilla::net diff --git a/netwerk/ipc/ProxyAutoConfigChild.h b/netwerk/ipc/ProxyAutoConfigChild.h index 8ba2867cbe82..a246fd3cfcfc 100644 --- a/netwerk/ipc/ProxyAutoConfigChild.h +++ b/netwerk/ipc/ProxyAutoConfigChild.h @@ -6,6 +6,8 @@ #ifndef ProxyAutoConfigChild_h__ #define ProxyAutoConfigChild_h__ +#include +#include "mozilla/LinkedList.h" #include "mozilla/net/PProxyAutoConfigChild.h" #include "mozilla/StaticPtr.h" #include "mozilla/UniquePtr.h" @@ -34,15 +36,43 @@ class ProxyAutoConfigChild final : public PProxyAutoConfigChild { const nsCString& aTestURI, const nsCString& aTestHost, GetProxyForURIResolver&& aResolver); - mozilla::ipc::IPCResult RecvGC(); - void Destroy(); private: virtual ~ProxyAutoConfigChild(); - void MainThreadActorDestroy(); + void ProcessPendingQ(); + bool ProcessPending(); + static void BindProxyAutoConfigChild( + RefPtr&& aActor, + Endpoint&& aEndpoint); UniquePtr mPAC; + bool mInProgress{false}; + bool mPACLoaded{false}; + bool mShutdown{false}; + + class PendingQuery final : public LinkedListElement> { + public: + NS_INLINE_DECL_REFCOUNTING(PendingQuery) + + explicit PendingQuery(const nsACString& aTestURI, + const nsACString& aTestHost, + GetProxyForURIResolver&& aResolver) + : mURI(aTestURI), mHost(aTestHost), mResolver(std::move(aResolver)) {} + + void Resolve(nsresult aStatus, const nsCString& aResult); + const nsCString& URI() const { return mURI; } + const nsCString& Host() const { return mHost; } + + private: + ~PendingQuery() = default; + + nsCString mURI; + nsCString mHost; + GetProxyForURIResolver mResolver; + }; + + LinkedList> mPendingQ; static StaticRefPtr sPACThread; static bool sShutdownObserverRegistered; diff --git a/netwerk/ipc/SocketProcessChild.cpp b/netwerk/ipc/SocketProcessChild.cpp index 79aefeec9c94..8e711a326e84 100644 --- a/netwerk/ipc/SocketProcessChild.cpp +++ b/netwerk/ipc/SocketProcessChild.cpp @@ -135,17 +135,6 @@ bool SocketProcessChild::Init(base::ProcessId aParentPid, return false; } - if (StaticPrefs::network_proxy_parse_pac_on_socket_process()) { - // For parsing PAC. - const char* jsInitFailureReason = JS_InitWithFailureDiagnostic(); - if (jsInitFailureReason) { - MOZ_CRASH_UNSAFE(jsInitFailureReason); - } - sInitializedJS = true; - - xpc::SelfHostedShmem::GetSingleton(); - } - BackgroundChild::Startup(); BackgroundChild::InitSocketStarter(this); @@ -690,6 +679,17 @@ mozilla::ipc::IPCResult SocketProcessChild::RecvGetHttpConnectionData( mozilla::ipc::IPCResult SocketProcessChild::RecvInitProxyAutoConfigChild( Endpoint&& aEndpoint) { + // For parsing PAC. + if (!sInitializedJS) { + const char* jsInitFailureReason = JS_InitWithFailureDiagnostic(); + if (jsInitFailureReason) { + MOZ_CRASH_UNSAFE(jsInitFailureReason); + } + sInitializedJS = true; + + xpc::SelfHostedShmem::GetSingleton(); + } + Unused << ProxyAutoConfigChild::Create(std::move(aEndpoint)); return IPC_OK(); } diff --git a/netwerk/test/unit/test_protocolproxyservice.js b/netwerk/test/unit/test_protocolproxyservice.js index e398252a687e..ddbaaed980b1 100644 --- a/netwerk/test/unit/test_protocolproxyservice.js +++ b/netwerk/test/unit/test_protocolproxyservice.js @@ -23,6 +23,7 @@ var ios = Services.io; var pps = Cc["@mozilla.org/network/protocol-proxy-service;1"].getService(); var prefs = Services.prefs; +var again = true; /** * Test nsIProtocolHandler that allows proxying, but doesn't allow HTTP @@ -921,6 +922,7 @@ function run_failed_script_test() { } var directFilter; +const TEST_URI = "http://127.0.0.1:7247/"; function failed_script_callback(pi) { // we should go direct @@ -937,7 +939,7 @@ function failed_script_callback(pi) { obs.addObserver(directFilterListener, "http-on-modify-request"); var ssm = Services.scriptSecurityManager; - let uri = "http://127.0.0.1:7247"; + let uri = TEST_URI; var chan = NetUtil.newChannel({ uri, loadingPrincipal: ssm.createContentPrincipal(Services.io.newURI(uri), {}), @@ -975,6 +977,10 @@ var directFilterListener = { subject instanceof Ci.nsIHttpChannel && subject instanceof Ci.nsIProxiedChannel ) { + info("check proxy in observe uri=" + subject.URI.spec); + if (subject.URI.spec != TEST_URI) { + return; + } check_proxy(subject.proxyInfo, "http", "127.0.0.1", 7246, 0, 0, false); this.onModifyRequestCalled = true; } @@ -1040,12 +1046,36 @@ function localhost_callback(pi) { Assert.equal(pi, null); // no proxy! prefs.setIntPref("network.proxy.type", 0); - do_test_finished(); + + // Due to the sandbox limitation on windows, we can't test this on windows. + if (mozinfo.socketprocess_networking && again && mozinfo.os != "win") { + info("run test again"); + again = false; + cleanUp(); + prefs.setBoolPref("network.proxy.parse_pac_on_socket_process", true); + run_filter_test(); + } else { + cleanUp(); + do_test_finished(); + } +} + +function cleanUp() { + prefs.clearUserPref("network.proxy.type"); + prefs.clearUserPref("network.proxy.http"); + prefs.clearUserPref("network.proxy.http_port"); + prefs.clearUserPref("network.proxy.socks"); + prefs.clearUserPref("network.proxy.socks_port"); + prefs.clearUserPref("network.proxy.autoconfig_url"); + prefs.clearUserPref("network.proxy.proxy_over_tls"); + prefs.clearUserPref("network.proxy.no_proxies_on"); + prefs.clearUserPref("network.proxy.parse_pac_on_socket_process"); } function run_test() { register_test_protocol_handler(); + prefs.setBoolPref("network.proxy.parse_pac_on_socket_process", false); // start of asynchronous test chain run_filter_test(); do_test_pending(); diff --git a/netwerk/test/unit/test_trr_proxy.js b/netwerk/test/unit/test_trr_proxy.js index 3a7b9189f427..37b91cb78ff2 100644 --- a/netwerk/test/unit/test_trr_proxy.js +++ b/netwerk/test/unit/test_trr_proxy.js @@ -19,6 +19,7 @@ const dns = Cc["@mozilla.org/network/dns-service;1"].getService( registerCleanupFunction(async () => { Services.prefs.clearUserPref("network.proxy.type"); + Services.prefs.clearUserPref("network.proxy.parse_pac_on_socket_process"); trr_clear_prefs(); }); @@ -47,7 +48,7 @@ const override = Cc["@mozilla.org/network/native-dns-override;1"].getService( Ci.nsINativeDNSResolverOverride ); -add_task(async function test_pac_dnsResolve() { +async function do_test_pac_dnsResolve() { Services.prefs.setCharPref("network.trr.confirmationNS", "skip"); Services.console.reset(); // Create a console listener. @@ -133,4 +134,27 @@ add_task(async function test_pac_dnsResolve() { await test_with("test3.com", 2, false); await test_with("test4.com", 3, false); await httpserv.stop(); +} + +add_task(async function test_pac_dnsResolve() { + Services.prefs.setBoolPref( + "network.proxy.parse_pac_on_socket_process", + false + ); + + await do_test_pac_dnsResolve(); + + // Due to the sandbox limitation on windows, we can't test this on windows. + if (mozinfo.socketprocess_networking && mozinfo.os != "win") { + info("run test again"); + Services.prefs.clearUserPref("network.proxy.type"); + trr_clear_prefs(); + Services.prefs.setBoolPref( + "network.proxy.parse_pac_on_socket_process", + true + ); + Services.prefs.setIntPref("network.proxy.type", 2); + Services.prefs.setIntPref("network.proxy.type", 0); + await do_test_pac_dnsResolve(); + } });