Bug 1146316 - Preserve the wrapper of sandboxes, so that we never try to call WrapObject on them. r=bz.
☠☠ backed out by 30ed797c2454 ☠ ☠
authorPeter Van der Beken <peterv@propagandism.org>
Wed, 11 Apr 2018 11:52:13 +0200
changeset 414760 1316c78daeb6fd1883b3c1688c2591ee02c4d3e5
parent 414759 2feb276e4fcceb7badbc4b634129111656c60f62
child 414761 719f7596c208eab1f2c8a1a250f1a6d2a7304d65
push id33876
push userdluca@mozilla.com
push dateFri, 20 Apr 2018 23:00:46 +0000
treeherdermozilla-central@39ccabfd7d07 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1146316
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 1146316 - Preserve the wrapper of sandboxes, so that we never try to call WrapObject on them. r=bz.
js/xpconnect/src/Sandbox.cpp
js/xpconnect/src/SandboxPrivate.h
js/xpconnect/src/XPCJSRuntime.cpp
--- a/js/xpconnect/src/Sandbox.cpp
+++ b/js/xpconnect/src/Sandbox.cpp
@@ -1046,21 +1046,18 @@ xpc::CreateSandboxObject(JSContext* cx, 
     // flag, but such a change would break code in subtle ways for minimal
     // benefit. So we just switch it off here.
     priv->wantXrays =
       AccessCheck::isChrome(sandbox) ? false : options.wantXrays;
 
     {
         JSAutoCompartment ac(cx, sandbox);
 
-        nsCOMPtr<nsIScriptObjectPrincipal> sbp =
-            new SandboxPrivate(principal, sandbox);
-
-        // Pass on ownership of sbp to |sandbox|.
-        JS_SetPrivate(sandbox, sbp.forget().take());
+        // This creates a SandboxPrivate and passes ownership of it to |sandbox|.
+        SandboxPrivate::Create(principal, sandbox);
 
         // Ensure |Object.prototype| is instantiated before prototype-
         // splicing below.
         if (!JS_GetObjectPrototype(cx, sandbox))
             return NS_ERROR_XPC_UNEXPECTED;
 
         if (options.proto) {
             bool ok = JS_WrapObject(cx, &options.proto);
--- a/js/xpconnect/src/SandboxPrivate.h
+++ b/js/xpconnect/src/SandboxPrivate.h
@@ -17,27 +17,38 @@
 
 
 class SandboxPrivate : public nsIGlobalObject,
                        public nsIScriptObjectPrincipal,
                        public nsSupportsWeakReference,
                        public nsWrapperCache
 {
 public:
-    SandboxPrivate(nsIPrincipal* principal, JSObject* global)
-        : mPrincipal(principal)
-    {
-        SetIsNotDOMBinding();
-        SetWrapper(global);
-    }
-
     NS_DECL_CYCLE_COLLECTING_ISUPPORTS
     NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_AMBIGUOUS(SandboxPrivate,
                                                            nsIGlobalObject)
 
+    static void Create(nsIPrincipal* principal, JS::Handle<JSObject*> global)
+    {
+        RefPtr<SandboxPrivate> sbp = new SandboxPrivate(principal);
+        sbp->SetWrapper(global);
+        sbp->PreserveWrapper(ToSupports(sbp.get()));
+
+        // Pass on ownership of sbp to |global|.
+        // The type used to cast to void needs to match the one in GetPrivate.
+        JS_SetPrivate(global, static_cast<nsIScriptObjectPrincipal*>(sbp.forget().take()));
+    }
+
+    static SandboxPrivate* GetPrivate(JSObject* obj)
+    {
+        // The type used to cast to void needs to match the one in Create.
+        return static_cast<SandboxPrivate*>(
+            static_cast<nsIScriptObjectPrincipal*>(JS_GetPrivate(obj)));
+    }
+
     nsIPrincipal* GetPrincipal() override
     {
         return mPrincipal;
     }
 
     JSObject* GetGlobalJSObject() override
     {
         return GetWrapper();
@@ -55,14 +66,21 @@ public:
 
     size_t ObjectMoved(JSObject* obj, JSObject* old)
     {
         UpdateWrapper(obj, old);
         return 0;
     }
 
 private:
-    virtual ~SandboxPrivate() { }
+    explicit SandboxPrivate(nsIPrincipal* principal)
+        : mPrincipal(principal)
+    {
+        SetIsNotDOMBinding();
+    }
+
+    virtual ~SandboxPrivate()
+    { }
 
     nsCOMPtr<nsIPrincipal> mPrincipal;
 };
 
 #endif // __SANDBOXPRIVATE_H__
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -3042,12 +3042,20 @@ XPCJSRuntime::InitSingletonScopes()
     rv = CreateSandboxObject(cx, &v, /* principal = */ nullptr, compilationScopeOptions);
     MOZ_RELEASE_ASSERT(NS_SUCCEEDED(rv));
     mCompilationScope = js::UncheckedUnwrap(&v.toObject());
 }
 
 void
 XPCJSRuntime::DeleteSingletonScopes()
 {
+    // We're pretty late in shutdown, so we call ReleaseWrapper on the scopes. This way
+    // the GC can collect them immediately, and we don't rely on the CC to clean up.
+    RefPtr<SandboxPrivate> sandbox = SandboxPrivate::GetPrivate(mUnprivilegedJunkScope);
+    sandbox->ReleaseWrapper(sandbox);
     mUnprivilegedJunkScope = nullptr;
+    sandbox = SandboxPrivate::GetPrivate(mPrivilegedJunkScope);
+    sandbox->ReleaseWrapper(sandbox);
     mPrivilegedJunkScope = nullptr;
+    sandbox = SandboxPrivate::GetPrivate(mCompilationScope);
+    sandbox->ReleaseWrapper(sandbox);
     mCompilationScope = nullptr;
 }