From 98c1de392c737fc012c113f7a41df36faea4670c Mon Sep 17 00:00:00 2001 From: "benjamin%smedbergs.us" Date: Mon, 31 Dec 2007 18:17:36 +0000 Subject: [PATCH] Bug 409433 - XPConnect should never allow GC off the main thread, r=mrbkap sr=jst a=luser --- dom/src/base/nsJSEnvironment.cpp | 2 -- js/src/xpconnect/idl/nsIXPConnect.idl | 5 +-- js/src/xpconnect/shell/xpcshell.cpp | 3 -- js/src/xpconnect/src/nsXPConnect.cpp | 46 ------------------------- js/src/xpconnect/src/xpcjsruntime.cpp | 49 +++++++++------------------ js/src/xpconnect/src/xpcprivate.h | 4 --- 6 files changed, 17 insertions(+), 92 deletions(-) diff --git a/dom/src/base/nsJSEnvironment.cpp b/dom/src/base/nsJSEnvironment.cpp index d4df05c5c6f..a59ecbfb51b 100644 --- a/dom/src/base/nsJSEnvironment.cpp +++ b/dom/src/base/nsJSEnvironment.cpp @@ -3649,8 +3649,6 @@ nsJSRuntime::Init() // Set these global xpconnect options... nsIXPConnect *xpc = nsContentUtils::XPConnect(); - xpc->SetCollectGarbageOnMainThreadOnly(PR_TRUE); - xpc->SetDeferReleasesUntilAfterGarbageCollection(PR_TRUE); nsContentUtils::RegisterPrefCallback("dom.max_script_run_time", MaxScriptRunTimePrefChangedCallback, diff --git a/js/src/xpconnect/idl/nsIXPConnect.idl b/js/src/xpconnect/idl/nsIXPConnect.idl index 3f6cb0c6995..ab3e6cc7242 100644 --- a/js/src/xpconnect/idl/nsIXPConnect.idl +++ b/js/src/xpconnect/idl/nsIXPConnect.idl @@ -447,7 +447,7 @@ interface nsIXPCFunctionThisTranslator : nsISupports { 0xbd, 0xd6, 0x0, 0x0, 0x64, 0x65, 0x73, 0x74 } } %} -[uuid(20df9082-5b83-416d-ba80-0422af516d57)] +[uuid(e2342762-0055-4900-838a-7ef1dddde4c2)] interface nsIXPConnect : nsISupports { %{ C++ @@ -659,9 +659,6 @@ interface nsIXPConnect : nsISupports in JSObjectPtr aScope, in nsIClassInfo aClassInfo); - attribute PRBool collectGarbageOnMainThreadOnly; - attribute PRBool deferReleasesUntilAfterGarbageCollection; - void releaseJSContext(in JSContextPtr aJSContext, in PRBool noGC); JSVal variantToJS(in JSContextPtr ctx, in JSObjectPtr scope, in nsIVariant value); diff --git a/js/src/xpconnect/shell/xpcshell.cpp b/js/src/xpconnect/shell/xpcshell.cpp index d4903f61efd..4ee52652ce9 100644 --- a/js/src/xpconnect/shell/xpcshell.cpp +++ b/js/src/xpconnect/shell/xpcshell.cpp @@ -1344,9 +1344,6 @@ main(int argc, char **argv, char **envp) nsRefPtr secman = new FullTrustSecMan(); xpc->SetSecurityManagerForJSContext(cx, secman, 0xFFFF); - // xpc->SetCollectGarbageOnMainThreadOnly(PR_TRUE); - // xpc->SetDeferReleasesUntilAfterGarbageCollection(PR_TRUE); - #ifndef XPCONNECT_STANDALONE // Fetch the system principal and store it away in a global, to use for // script compilation in Load() and ProcessFile() (including interactive diff --git a/js/src/xpconnect/src/nsXPConnect.cpp b/js/src/xpconnect/src/nsXPConnect.cpp index 4a04b06bfa7..82a9a1b3dcb 100644 --- a/js/src/xpconnect/src/nsXPConnect.cpp +++ b/js/src/xpconnect/src/nsXPConnect.cpp @@ -1976,52 +1976,6 @@ nsXPConnect::UpdateXOWs(JSContext* aJSContext, return NS_OK; } -/* attribute PRBool collectGarbageOnMainThreadOnly; */ -NS_IMETHODIMP -nsXPConnect::GetCollectGarbageOnMainThreadOnly(PRBool *aCollectGarbageOnMainThreadOnly) -{ - XPCJSRuntime* rt = GetRuntime(); - if(!rt) - return UnexpectedFailure(NS_ERROR_FAILURE); - - *aCollectGarbageOnMainThreadOnly = rt->GetMainThreadOnlyGC(); - return NS_OK; -} - -NS_IMETHODIMP -nsXPConnect::SetCollectGarbageOnMainThreadOnly(PRBool aCollectGarbageOnMainThreadOnly) -{ - XPCJSRuntime* rt = GetRuntime(); - if(!rt) - return UnexpectedFailure(NS_ERROR_FAILURE); - - rt->SetMainThreadOnlyGC(aCollectGarbageOnMainThreadOnly); - return NS_OK; -} - -/* attribute PRBool deferReleasesUntilAfterGarbageCollection; */ -NS_IMETHODIMP -nsXPConnect::GetDeferReleasesUntilAfterGarbageCollection(PRBool *aDeferReleasesUntilAfterGarbageCollection) -{ - XPCJSRuntime* rt = GetRuntime(); - if(!rt) - return UnexpectedFailure(NS_ERROR_FAILURE); - - *aDeferReleasesUntilAfterGarbageCollection = rt->GetDeferReleases(); - return NS_OK; -} - -NS_IMETHODIMP -nsXPConnect::SetDeferReleasesUntilAfterGarbageCollection(PRBool aDeferReleasesUntilAfterGarbageCollection) -{ - XPCJSRuntime* rt = GetRuntime(); - if(!rt) - return UnexpectedFailure(NS_ERROR_FAILURE); - - rt->SetDeferReleases(aDeferReleasesUntilAfterGarbageCollection); - return NS_OK; -} - /* void releaseJSContext (in JSContextPtr aJSContext, in PRBool noGC); */ NS_IMETHODIMP nsXPConnect::ReleaseJSContext(JSContext * aJSContext, PRBool noGC) diff --git a/js/src/xpconnect/src/xpcjsruntime.cpp b/js/src/xpconnect/src/xpcjsruntime.cpp index 815ae0f20bf..531f760c67f 100644 --- a/js/src/xpconnect/src/xpcjsruntime.cpp +++ b/js/src/xpconnect/src/xpcjsruntime.cpp @@ -526,7 +526,7 @@ JSBool XPCJSRuntime::GCCallback(JSContext *cx, JSGCStatus status) { case JSGC_BEGIN: { - if(self->GetMainThreadOnlyGC() && !NS_IsMainThread()) + if(!NS_IsMainThread()) { return JS_FALSE; } @@ -544,11 +544,8 @@ JSBool XPCJSRuntime::GCCallback(JSContext *cx, JSGCStatus status) } dyingWrappedJSArray = &self->mWrappedJSToReleaseArray; - { - XPCLock* lock = self->GetMainThreadOnlyGC() ? - nsnull : self->GetMapLock(); - XPCAutoLock al(lock); // lock the wrapper map if necessary + { JSDyingJSObjectData data = {cx, dyingWrappedJSArray}; // Add any wrappers whose JSObjects are to be finalized to @@ -585,23 +582,18 @@ JSBool XPCJSRuntime::GCCallback(JSContext *cx, JSGCStatus status) // to be dead. dyingWrappedJSArray = &self->mWrappedJSToReleaseArray; - XPCLock* mapLock = self->GetMainThreadOnlyGC() ? - nsnull : self->GetMapLock(); while(1) { nsXPCWrappedJS* wrapper; + PRInt32 count = dyingWrappedJSArray->Count(); + if(!count) { - XPCAutoLock al(mapLock); // lock if necessary - PRInt32 count = dyingWrappedJSArray->Count(); - if(!count) - { - dyingWrappedJSArray->Compact(); - break; - } - wrapper = static_cast - (dyingWrappedJSArray->ElementAt(count-1)); - dyingWrappedJSArray->RemoveElementAt(count-1); + dyingWrappedJSArray->Compact(); + break; } + wrapper = static_cast + (dyingWrappedJSArray->ElementAt(count-1)); + dyingWrappedJSArray->RemoveElementAt(count-1); NS_RELEASE(wrapper); } @@ -801,9 +793,6 @@ JSBool XPCJSRuntime::GCCallback(JSContext *cx, JSGCStatus status) // the js engine. So this could be simultaneous with the // events above. - XPCLock* lock = self->GetMainThreadOnlyGC() ? - nsnull : self->GetMapLock(); - // Do any deferred released of native objects. if(self->GetDeferReleases()) { @@ -816,7 +805,6 @@ JSBool XPCJSRuntime::GCCallback(JSContext *cx, JSGCStatus status) { nsISupports* obj; { - XPCAutoLock al(lock); // lock if necessary PRInt32 count = array->Count(); if(!count) { @@ -1067,7 +1055,6 @@ XPCJSRuntime::XPCJSRuntime(nsXPConnect* aXPConnect, mThreadRunningGC(nsnull), mWrappedJSToReleaseArray(), mNativesToReleaseArray(), - mMainThreadOnlyGC(JS_FALSE), mDeferReleases(JS_FALSE), mDoingFinalization(JS_FALSE), mVariantRoots(nsnull), @@ -1273,18 +1260,14 @@ XPCJSRuntime::DeferredRelease(nsISupports* obj) NS_ASSERTION(obj, "bad param"); NS_ASSERTION(GetDeferReleases(), "bad call"); - XPCLock* lock = GetMainThreadOnlyGC() ? nsnull : GetMapLock(); + if(!mNativesToReleaseArray.Count()) { - XPCAutoLock al(lock); // lock if necessary - if(!mNativesToReleaseArray.Count()) - { - // This array sometimes has 1000's - // of entries, and usually has 50-200 entries. Avoid lots - // of incremental grows. We compact it down when we're done. - mNativesToReleaseArray.SizeTo(256); - } - return mNativesToReleaseArray.AppendElement(obj); - } + // This array sometimes has 1000's + // of entries, and usually has 50-200 entries. Avoid lots + // of incremental grows. We compact it down when we're done. + mNativesToReleaseArray.SizeTo(256); + } + return mNativesToReleaseArray.AppendElement(obj); } /***************************************************************************/ diff --git a/js/src/xpconnect/src/xpcprivate.h b/js/src/xpconnect/src/xpcprivate.h index f062c5a5803..d3c67164bc7 100644 --- a/js/src/xpconnect/src/xpcprivate.h +++ b/js/src/xpconnect/src/xpcprivate.h @@ -656,9 +656,6 @@ public: XPCContext* GetXPCContext(JSContext* cx); XPCContext* SyncXPCContextList(JSContext* cx = nsnull); - JSBool GetMainThreadOnlyGC() const {return mMainThreadOnlyGC;} - void SetMainThreadOnlyGC(JSBool b) {mMainThreadOnlyGC = b;} - JSBool GetDeferReleases() const {return mDeferReleases;} void SetDeferReleases(JSBool b) {/* If deferring is turned off while any are pending they'll leak! */ @@ -790,7 +787,6 @@ private: PRThread* mThreadRunningGC; nsVoidArray mWrappedJSToReleaseArray; nsVoidArray mNativesToReleaseArray; - JSBool mMainThreadOnlyGC; JSBool mDeferReleases; JSBool mDoingFinalization; XPCRootSetElem *mVariantRoots;