Bug 1366133 - Part 1: make nsISystemProxySettings::GetPACURI happen in another thread; r?bagder
MozReview-Commit-ID: FyiNM8KX0gk
--- 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;
}