From 8443f4d14c86b6e3136e82181ff24f364e44ea7d Mon Sep 17 00:00:00 2001 From: Peter Van der Beken Date: Thu, 7 May 2009 11:19:36 -0700 Subject: [PATCH] Fix for bug 475737 (Windows stay alive too long because nsJSContext doesn't unlink correctly). r=bent, sr=jst. --- dom/base/nsJSEnvironment.cpp | 35 ++++++++++++++++------- dom/base/nsJSEnvironment.h | 7 +++-- js/src/jscntxt.cpp | 4 +++ js/src/xpconnect/src/xpcjsruntime.cpp | 1 - xpcom/base/nsAgg.h | 2 +- xpcom/glue/nsCycleCollectionParticipant.h | 17 ++++++----- 6 files changed, 45 insertions(+), 21 deletions(-) diff --git a/dom/base/nsJSEnvironment.cpp b/dom/base/nsJSEnvironment.cpp index 9f721af9ba40..282ef836bb6f 100644 --- a/dom/base/nsJSEnvironment.cpp +++ b/dom/base/nsJSEnvironment.cpp @@ -1286,7 +1286,9 @@ nsJSContext::~nsJSContext() #endif NS_PRECONDITION(!mTerminations, "Shouldn't have termination funcs by now"); - Unlink(); + mGlobalWrapperRef = nsnull; + + DestroyJSContext(); --sContextCount; @@ -1303,7 +1305,7 @@ nsJSContext::~nsJSContext() } void -nsJSContext::Unlink() +nsJSContext::DestroyJSContext() { if (!mContext) return; @@ -1316,30 +1318,37 @@ nsJSContext::Unlink() JSOptionChangedCallback, this); - // Release mGlobalWrapperRef before the context is destroyed - mGlobalWrapperRef = nsnull; + PRBool do_gc = mGCOnDestruction && !sGCTimer && sReadyForGC; // Let xpconnect destroy the JSContext when it thinks the time is right. nsIXPConnect *xpc = nsContentUtils::XPConnect(); if (xpc) { - PRBool do_gc = mGCOnDestruction && !sGCTimer && sReadyForGC; - xpc->ReleaseJSContext(mContext, !do_gc); - } else { + } else if (do_gc) { ::JS_DestroyContext(mContext); + } else { + ::JS_DestroyContextNoGC(mContext); } mContext = nsnull; } // QueryInterface implementation for nsJSContext NS_IMPL_CYCLE_COLLECTION_CLASS(nsJSContext) +NS_IMPL_CYCLE_COLLECTION_ROOT_BEGIN(nsJSContext) + NS_ASSERTION(!tmp->mContext || tmp->mContext->outstandingRequests == 0, + "Trying to unlink a context with outstanding requests."); + tmp->mIsInitialized = PR_FALSE; + tmp->mGCOnDestruction = PR_FALSE; + tmp->DestroyJSContext(); +NS_IMPL_CYCLE_COLLECTION_ROOT_END +NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(nsJSContext) +NS_IMPL_CYCLE_COLLECTION_TRACE_END NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsJSContext) NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mGlobalWrapperRef) - tmp->Unlink(); - tmp->mIsInitialized = PR_FALSE; NS_IMPL_CYCLE_COLLECTION_UNLINK_END -NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsJSContext) +NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_REFCNT(nsJSContext, tmp->GetCCRefcnt()) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mGlobalWrapperRef) + NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mContext"); nsContentUtils::XPConnect()->NoteJSContext(tmp->mContext, cb); NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END @@ -1353,6 +1362,12 @@ NS_INTERFACE_MAP_END NS_IMPL_CYCLE_COLLECTING_ADDREF_AMBIGUOUS(nsJSContext, nsIScriptContext) NS_IMPL_CYCLE_COLLECTING_RELEASE_AMBIGUOUS(nsJSContext, nsIScriptContext) +nsrefcnt +nsJSContext::GetCCRefcnt() +{ + return mRefCnt.get() + mContext->outstandingRequests; +} + nsresult nsJSContext::EvaluateStringWithValue(const nsAString& aScript, void *aScopeObject, diff --git a/dom/base/nsJSEnvironment.h b/dom/base/nsJSEnvironment.h index 45d255bc3327..2c8603193a4a 100644 --- a/dom/base/nsJSEnvironment.h +++ b/dom/base/nsJSEnvironment.h @@ -57,7 +57,8 @@ public: virtual ~nsJSContext(); NS_DECL_CYCLE_COLLECTING_ISUPPORTS - NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(nsJSContext, nsIScriptContext) + NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_AMBIGUOUS(nsJSContext, + nsIScriptContext) virtual PRUint32 GetScriptTypeID() { return nsIProgrammingLanguage::JAVASCRIPT; } @@ -218,7 +219,9 @@ protected: // related to our exception. void ReportPendingException(PRBool aSetAsideFrameChain); private: - void Unlink(); + void DestroyJSContext(); + + nsrefcnt GetCCRefcnt(); JSContext *mContext; PRUint32 mNumEvaluations; diff --git a/js/src/jscntxt.cpp b/js/src/jscntxt.cpp index af99fac87e50..caa8c78609d2 100644 --- a/js/src/jscntxt.cpp +++ b/js/src/jscntxt.cpp @@ -594,6 +594,8 @@ js_DestroyContext(JSContext *cx, JSDestroyContextMode mode) #endif rt = cx->runtime; + JS_ASSERT_IF(rt->gcRunning, cx->outstandingRequests == 0); + if (mode != JSDCM_NEW_FAILED) { cxCallback = rt->cxCallback; if (cxCallback) { @@ -629,6 +631,8 @@ js_DestroyContext(JSContext *cx, JSDestroyContextMode mode) || cx->requestDepth != 0 #endif ) { + JS_ASSERT(rt->gcLevel == 0); + JS_UNLOCK_GC(rt); if (last) { diff --git a/js/src/xpconnect/src/xpcjsruntime.cpp b/js/src/xpconnect/src/xpcjsruntime.cpp index 08a9e7cbcfde..7872ce782147 100644 --- a/js/src/xpconnect/src/xpcjsruntime.cpp +++ b/js/src/xpconnect/src/xpcjsruntime.cpp @@ -371,7 +371,6 @@ void XPCJSRuntime::TraceXPConnectRoots(JSTracer *trc, JSBool rootGlobals) } } } - NS_ASSERTION(!rootGlobals || mUnrootedGlobalCount == 0, "bad state"); } XPCWrappedNativeScope::TraceJS(trc, this); diff --git a/xpcom/base/nsAgg.h b/xpcom/base/nsAgg.h index a7589756bbc9..0fe853339524 100644 --- a/xpcom/base/nsAgg.h +++ b/xpcom/base/nsAgg.h @@ -309,7 +309,7 @@ _class::AggregatedQueryInterface(REFNSIID aIID, void** aInstancePtr) \ "not the nsISupports pointer we expect"); \ _class *tmp = static_cast<_class*>(Downcast(s)); \ if (!tmp->IsPartOfAggregated()) \ - NS_IMPL_CYCLE_COLLECTION_DESCRIBE(_class) + NS_IMPL_CYCLE_COLLECTION_DESCRIBE(_class, tmp->mRefCnt.get()) #define NS_GENERIC_AGGREGATED_CONSTRUCTOR(_InstanceClass) \ static NS_METHOD \ diff --git a/xpcom/glue/nsCycleCollectionParticipant.h b/xpcom/glue/nsCycleCollectionParticipant.h index 420a9d0a0817..ef773fabfc79 100644 --- a/xpcom/glue/nsCycleCollectionParticipant.h +++ b/xpcom/glue/nsCycleCollectionParticipant.h @@ -322,14 +322,14 @@ public: /////////////////////////////////////////////////////////////////////////////// #ifdef DEBUG_CC -#define NS_IMPL_CYCLE_COLLECTION_DESCRIBE(_class) \ - cb.DescribeNode(RefCounted, tmp->mRefCnt.get(), sizeof(_class), #_class); +#define NS_IMPL_CYCLE_COLLECTION_DESCRIBE(_class, _refcnt) \ + cb.DescribeNode(RefCounted, _refcnt, sizeof(_class), #_class); #else -#define NS_IMPL_CYCLE_COLLECTION_DESCRIBE(_class) \ - cb.DescribeNode(RefCounted, tmp->mRefCnt.get()); +#define NS_IMPL_CYCLE_COLLECTION_DESCRIBE(_class, _refcnt) \ + cb.DescribeNode(RefCounted, _refcnt); #endif -#define NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(_class) \ +#define NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_REFCNT(_class, _refcnt) \ NS_IMETHODIMP \ NS_CYCLE_COLLECTION_CLASSNAME(_class)::Traverse \ (void *p, \ @@ -339,7 +339,10 @@ public: NS_ASSERTION(CheckForRightISupports(s), \ "not the nsISupports pointer we expect"); \ _class *tmp = Downcast(s); \ - NS_IMPL_CYCLE_COLLECTION_DESCRIBE(_class) + NS_IMPL_CYCLE_COLLECTION_DESCRIBE(_class, _refcnt) + +#define NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(_class) \ + NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_REFCNT(_class, tmp->mRefCnt.get()) // Base class' CC participant should return NS_SUCCESS_INTERRUPTED_TRAVERSE // from Traverse if it wants derived classes to not traverse anything from @@ -369,7 +372,7 @@ public: nsCycleCollectionTraversalCallback &cb) \ { \ _class *tmp = static_cast<_class*>(p); \ - NS_IMPL_CYCLE_COLLECTION_DESCRIBE(_class) + NS_IMPL_CYCLE_COLLECTION_DESCRIBE(_class, tmp->mRefCnt.get()) #ifdef DEBUG_CC #define NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(_cb, _name) \