Bug 811206, Fix JSHolder drop handling, part 2 (assert), r=mccr8
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Wed, 28 Nov 2012 02:56:06 +0200
changeset 114358 aaf77faa6886e2bdc344e94d5a9d25eb86098dde
parent 114357 a196bff3a967b4d54db77fc57a823c37a4b02bac
child 114359 6c23f41b074768a4970c493edeafedee43450a1e
push id18716
push usereakhgari@mozilla.com
push dateWed, 28 Nov 2012 14:43:36 +0000
treeherdermozilla-inbound@33b2ec1bc7f3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs811206
milestone20.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 811206, Fix JSHolder drop handling, part 2 (assert), r=mccr8
accessible/src/msaa/nsAccessNodeWrap.cpp
accessible/src/msaa/nsAccessNodeWrap.h
js/xpconnect/src/XPCJSRuntime.cpp
js/xpconnect/src/nsXPConnect.cpp
js/xpconnect/src/xpcprivate.h
xpcom/base/nsCycleCollector.cpp
xpcom/base/nsCycleCollector.h
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -254,19 +254,45 @@ XPCJSRuntime::AddJSHolder(void* aHolder,
     mJSHolders.Put(aHolder, aTracer);
     if (wasEmpty && mJSHolders.Count() == 1) {
       nsLayoutStatics::AddRef();
     }
 
     return NS_OK;
 }
 
+#ifdef DEBUG
+static void
+AssertNoGcThing(void* aGCThing, const char* aName, void* aClosure)
+{
+    MOZ_ASSERT(!aGCThing);
+}
+
+void
+XPCJSRuntime::AssertNoObjectsToTrace(void* aPossibleJSHolder)
+{
+    nsScriptObjectTracer* tracer = mJSHolders.Get(aPossibleJSHolder);
+    if (tracer && tracer->Trace) {
+        tracer->Trace(aPossibleJSHolder, AssertNoGcThing, nullptr);
+    }
+}
+#endif
+
 nsresult
 XPCJSRuntime::RemoveJSHolder(void* aHolder)
 {
+#ifdef DEBUG
+    // Assert that the holder doesn't try to keep any GC things alive.
+    // In case of unlinking cycle collector calls AssertNoObjectsToTrace
+    // manually because we don't want to check the holder before we are
+    // finished unlinking it
+    if (aHolder != mObjectToUnlink) {
+        AssertNoObjectsToTrace(aHolder);
+    }
+#endif
     bool hadOne = mJSHolders.Count() == 1;
     mJSHolders.Remove(aHolder);
     if (hadOne && mJSHolders.Count() == 0) {
       nsLayoutStatics::Release();
     }
 
     return NS_OK;
 }
@@ -2406,16 +2432,19 @@ XPCJSRuntime::XPCJSRuntime(nsXPConnect* 
    mWrappedJSRoots(nullptr),
    mObjectHolderRoots(nullptr),
    mWatchdogLock(nullptr),
    mWatchdogWakeup(nullptr),
    mWatchdogThread(nullptr),
    mWatchdogHibernating(false),
    mLastActiveTime(-1),
    mExceptionManagerNotAvailable(false)
+#ifdef DEBUG
+   , mObjectToUnlink(nullptr)
+#endif
 {
 #ifdef XPC_CHECK_WRAPPERS_AT_SHUTDOWN
     DEBUG_WrappedNativeHashtable =
         JS_NewDHashTable(JS_DHashGetStubOps(), nullptr,
                          sizeof(JSDHashEntryStub), 128);
 #endif
 
     DOM_InitInterfaces();
--- a/js/xpconnect/src/nsXPConnect.cpp
+++ b/js/xpconnect/src/nsXPConnect.cpp
@@ -2708,16 +2708,32 @@ nsXPConnect::WriteFunction(nsIObjectOutp
 }
 
 NS_IMETHODIMP
 nsXPConnect::ReadFunction(nsIObjectInputStream *stream, JSContext *cx, JSObject **functionObjp)
 {
     return ReadScriptOrFunction(stream, cx, nullptr, functionObjp);
 }
 
+#ifdef DEBUG
+void
+nsXPConnect::SetObjectToUnlink(void* aObject)
+{
+    if (mRuntime)
+        mRuntime->SetObjectToUnlink(aObject);
+}
+
+void
+nsXPConnect::AssertNoObjectsToTrace(void* aPossibleJSHolder)
+{
+    if (mRuntime)
+        mRuntime->AssertNoObjectsToTrace(aPossibleJSHolder);
+}
+#endif
+
 /* These are here to be callable from a debugger */
 JS_BEGIN_EXTERN_C
 JS_EXPORT_API(void) DumpJSStack()
 {
     nsresult rv;
     nsCOMPtr<nsIXPConnect> xpc(do_GetService(nsIXPConnect::GetCID(), &rv));
     if (NS_SUCCEEDED(rv) && xpc)
         xpc->DebugDumpJSStack(true, true, false);
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -480,16 +480,21 @@ public:
     // non-interface implementation
 public:
     // These get non-addref'd pointers
     static nsXPConnect*  GetXPConnect();
     static nsXPConnect*  FastGetXPConnect() { return gSelf ? gSelf : GetXPConnect(); }
     static XPCJSRuntime* GetRuntimeInstance();
     XPCJSRuntime* GetRuntime() {return mRuntime;}
 
+#ifdef DEBUG
+    void SetObjectToUnlink(void* aObject);
+    void AssertNoObjectsToTrace(void* aPossibleJSHolder);
+#endif
+
     // Gets addref'd pointer
     static nsresult GetInterfaceInfoManager(nsIInterfaceInfoSuperManager** iim,
                                             nsXPConnect* xpc = nullptr);
 
     static JSBool IsISupportsDescendant(nsIInterfaceInfo* info);
 
     nsIXPCSecurityManager* GetDefaultSecurityManager() const
     {
@@ -820,16 +825,20 @@ public:
 
     inline void AddVariantRoot(XPCTraceableVariant* variant);
     inline void AddWrappedJSRoot(nsXPCWrappedJS* wrappedJS);
     inline void AddObjectHolderRoot(XPCJSObjectHolder* holder);
 
     nsresult AddJSHolder(void* aHolder, nsScriptObjectTracer* aTracer);
     nsresult RemoveJSHolder(void* aHolder);
     nsresult TestJSHolder(void* aHolder, bool* aRetval);
+#ifdef DEBUG
+    void SetObjectToUnlink(void* aObject) { mObjectToUnlink = aObject; }
+    void AssertNoObjectsToTrace(void* aPossibleJSHolder);
+#endif
 
     static void SuspectWrappedNative(XPCWrappedNative *wrapper,
                                      nsCycleCollectionTraversalCallback &cb);
 
     void DebugDump(int16_t depth);
 
     void SystemIsBeingShutDown();
 
@@ -985,16 +994,20 @@ private:
         mozilla::AlignedStorage2<XPCReadableJSStringWrapper> mString;
         bool mInUse;
     };
 
     StringWrapperEntry mScratchStrings[XPCCCX_STRING_CACHE_SIZE];
 
     friend class AutoLockWatchdog;
     friend class XPCIncrementalReleaseRunnable;
+
+#ifdef DEBUG
+    void* mObjectToUnlink;
+#endif
 };
 
 /***************************************************************************/
 /***************************************************************************/
 // XPCContext is mostly a dumb class to hold JSContext specific data and
 // maps that let us find wrappers created for the given JSContext.
 
 // no virtuals
--- a/xpcom/base/nsCycleCollector.cpp
+++ b/xpcom/base/nsCycleCollector.cpp
@@ -2376,17 +2376,28 @@ nsCycleCollector::CollectWhite(nsICycleC
             PtrInfo *pinfo = mWhiteNodes->ElementAt(i);
             aListener->DescribeGarbage((uint64_t)pinfo->mPointer);
         }
         aListener->End();
     }
 
     for (uint32_t i = 0; i < count; ++i) {
         PtrInfo *pinfo = mWhiteNodes->ElementAt(i);
+#ifdef DEBUG
+        if (mJSRuntime) {
+            mJSRuntime->SetObjectToUnlink(pinfo->mPointer);
+        }
+#endif
         rv = pinfo->mParticipant->Unlink(pinfo->mPointer);
+#ifdef DEBUG
+        if (mJSRuntime) {
+            mJSRuntime->SetObjectToUnlink(nullptr);
+            mJSRuntime->AssertNoObjectsToTrace(pinfo->mPointer);
+        }
+#endif
         if (NS_FAILED(rv)) {
             Fault("Failed unlink call while unlinking", pinfo);
 #ifdef DEBUG_CC
             mStats.mFailedUnlink++;
 #endif
         }
         else {
 #ifdef DEBUG_CC
--- a/xpcom/base/nsCycleCollector.h
+++ b/xpcom/base/nsCycleCollector.h
@@ -76,16 +76,21 @@ struct nsCycleCollectionJSRuntime
      * Runs the JavaScript GC. |reason| is a gcreason::Reason from jsfriendapi.h.
      */
     virtual void Collect(uint32_t reason) = 0;
 
     /**
      * Get the JS cycle collection participant.
      */
     virtual nsCycleCollectionParticipant *GetParticipant() = 0;
+
+#ifdef DEBUG
+    virtual void SetObjectToUnlink(void* aObject) = 0;
+    virtual void AssertNoObjectsToTrace(void* aPossibleJSHolder) = 0;
+#endif
 };
 
 // Helpers for interacting with JS
 void nsCycleCollector_registerJSRuntime(nsCycleCollectionJSRuntime *rt);
 void nsCycleCollector_forgetJSRuntime();
 
 #ifdef DEBUG
 void nsCycleCollector_DEBUG_shouldBeFreed(nsISupports *n);