Bug 1419373 - Call ShutdownMarker() on all JSObject2WrappedJSMaps at the same time r=mccr8
authorJon Coppeard <jcoppeard@mozilla.com>
Mon, 23 Apr 2018 10:51:33 +0100
changeset 468627 d1c67d1afe266d91994533cf9e31e3ead393efe8
parent 468626 dcfb7f0bb6a9171b8bcffaf488f152680ed82b65
child 468628 dd5dc2aea4933707859270e3a1a904c3241b4641
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs1419373
milestone61.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 1419373 - Call ShutdownMarker() on all JSObject2WrappedJSMaps at the same time r=mccr8
js/xpconnect/src/XPCJSRuntime.cpp
js/xpconnect/src/XPCWrappedJS.cpp
js/xpconnect/src/XPCWrappedNativeScope.cpp
js/xpconnect/src/nsXPConnect.cpp
js/xpconnect/src/xpcprivate.h
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -182,18 +182,23 @@ CompartmentPrivate::CompartmentPrivate(J
 {
     MOZ_COUNT_CTOR(xpc::CompartmentPrivate);
     mozilla::PodArrayZero(wrapperDenialWarnings);
 }
 
 CompartmentPrivate::~CompartmentPrivate()
 {
     MOZ_COUNT_DTOR(xpc::CompartmentPrivate);
+    delete mWrappedJSMap;
+}
+
+void
+CompartmentPrivate::SystemIsBeingShutDown()
+{
     mWrappedJSMap->ShutdownMarker();
-    delete mWrappedJSMap;
 }
 
 RealmPrivate::RealmPrivate(JS::Realm* realm)
     : scriptability(JS::GetCompartmentForRealm(realm))
     , scope(nullptr)
 {
 }
 
@@ -1013,33 +1018,37 @@ CompartmentPrivate::SizeOfIncludingThis(
     n += mWrappedJSMap->SizeOfIncludingThis(mallocSizeOf);
     n += mWrappedJSMap->SizeOfWrappedJS(mallocSizeOf);
     return n;
 }
 
 /***************************************************************************/
 
 void
+XPCJSRuntime::SystemIsBeingShutDown()
+{
+    // We don't want to track wrapped JS roots after this point since we're
+    // making them !IsValid anyway through SystemIsBeingShutDown.
+    mWrappedJSRoots = nullptr;
+}
+
+void
 XPCJSRuntime::Shutdown(JSContext* cx)
 {
     // This destructor runs before ~CycleCollectedJSContext, which does the
     // actual JS_DestroyContext() call. But destroying the context triggers
     // one final GC, which can call back into the context with various
     // callbacks if we aren't careful. Null out the relevant callbacks.
     JS_RemoveFinalizeCallback(cx, FinalizeCallback);
     JS_RemoveWeakPointerZonesCallback(cx, WeakPointerZonesCallback);
     JS_RemoveWeakPointerCompartmentCallback(cx, WeakPointerCompartmentCallback);
     xpc_DelocalizeRuntime(JS_GetRuntime(cx));
 
     JS::SetGCSliceCallback(cx, mPrevGCSliceCallback);
 
-    // We don't want to track wrapped JS roots after this point since we're
-    // making them !IsValid anyway through SystemIsBeingShutDown.
-    mWrappedJSRoots = nullptr;
-
     // clean up and destroy maps...
     mWrappedJSMap->ShutdownMarker();
     delete mWrappedJSMap;
     mWrappedJSMap = nullptr;
 
     delete mWrappedJSClassMap;
     mWrappedJSClassMap = nullptr;
 
--- a/js/xpconnect/src/XPCWrappedJS.cpp
+++ b/js/xpconnect/src/XPCWrappedJS.cpp
@@ -629,19 +629,21 @@ nsXPCWrappedJS::SystemIsBeingShutDown()
 
     // mJSObj == nullptr is used to indicate that the wrapper is no longer valid
     // and that calls should fail without trying to use any of the
     // xpconnect mechanisms. 'IsValid' is implemented by checking this pointer.
 
     // NOTE: that mClass is retained so that GetInterfaceInfo can continue to
     // work (and avoid crashing some platforms).
 
-    // Use of unsafeGet() is to avoid triggering post barriers in shutdown, as
-    // this will access the chunk containing mJSObj, which may have been freed
-    // at this point.
+    // Clear the contents of the pointer using unsafeGet() to avoid
+    // triggering post barriers in shutdown, as this will access the chunk
+    // containing mJSObj, which may have been freed at this point. This is safe
+    // if we are not currently running an incremental GC.
+    MOZ_ASSERT(!IsIncrementalGCInProgress(xpc_GetSafeJSContext()));
     *mJSObj.unsafeGet() = nullptr;
 
     // Notify other wrappers in the chain.
     if (mNext)
         mNext->SystemIsBeingShutDown();
 }
 
 size_t
--- a/js/xpconnect/src/XPCWrappedNativeScope.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeScope.cpp
@@ -544,16 +544,19 @@ XPCWrappedNativeScope::SystemIsBeingShut
         for (auto i = cur->mWrappedNativeMap->Iter(); !i.Done(); i.Next()) {
             auto entry = static_cast<Native2WrappedNativeMap::Entry*>(i.Get());
             XPCWrappedNative* wrapper = entry->value;
             if (wrapper->IsValid()) {
                 wrapper->SystemIsBeingShutDown();
             }
             i.Remove();
         }
+
+        CompartmentPrivate* priv = CompartmentPrivate::Get(cur->Compartment());
+        priv->SystemIsBeingShutDown();
     }
 
     // Now it is safe to kill all the scopes.
     KillDyingScopes();
 }
 
 
 /***************************************************************************/
--- a/js/xpconnect/src/nsXPConnect.cpp
+++ b/js/xpconnect/src/nsXPConnect.cpp
@@ -91,16 +91,17 @@ nsXPConnect::~nsXPConnect()
     // we unrooted above), and once after forcing a bunch of shutdown in
     // XPConnect, to clean the stuff we forcibly disconnected. The forced
     // shutdown code defaults to leaking in a number of situations, so we can't
     // get by with only the second GC. :-(
     mRuntime->GarbageCollect(JS::gcreason::XPCONNECT_SHUTDOWN);
 
     mShuttingDown = true;
     XPCWrappedNativeScope::SystemIsBeingShutDown();
+    mRuntime->SystemIsBeingShutDown();
 
     // The above causes us to clean up a bunch of XPConnect data structures,
     // after which point we need to GC to clean everything up. We need to do
     // this before deleting the XPCJSContext, because doing so destroys the
     // maps that our finalize callback depends on.
     mRuntime->GarbageCollect(JS::gcreason::XPCONNECT_SHUTDOWN);
 
     NS_RELEASE(gSystemPrincipal);
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -627,16 +627,18 @@ public:
 
     JSObject* UnprivilegedJunkScope() { return mUnprivilegedJunkScope; }
     JSObject* PrivilegedJunkScope() { return mPrivilegedJunkScope; }
     JSObject* CompilationScope() { return mCompilationScope; }
 
     void InitSingletonScopes();
     void DeleteSingletonScopes();
 
+    void SystemIsBeingShutDown();
+
 private:
     explicit XPCJSRuntime(JSContext* aCx);
 
     MOZ_IS_CLASS_INIT
     void Initialize(JSContext* cx);
     void Shutdown(JSContext* cx) override;
 
     void ReleaseIncrementally(nsTArray<nsISupports*>& array);
@@ -3027,16 +3029,18 @@ public:
         if (locationURI)
             return;
         locationURI = aLocationURI;
     }
 
     JSObject2WrappedJSMap* GetWrappedJSMap() const { return mWrappedJSMap; }
     void UpdateWeakPointersAfterGC();
 
+    void SystemIsBeingShutDown();
+
     size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf);
 
 private:
     nsCString location;
     nsCOMPtr<nsIURI> locationURI;
     JSObject2WrappedJSMap* mWrappedJSMap;
 
     bool TryParseLocationURI(LocationHint aType, nsIURI** aURI);