Bug 1524946 - Ensure script private value stays alive during dynamic module import r=jandem a=dveditz
☠☠ backed out by 6370b90702b7 ☠ ☠
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 08 Feb 2019 18:39:54 +0000
changeset 458376 faded33ed22f63a16ed902c2f3e41b39cb4371f3
parent 458375 65ed3025c27100c4ccaa3138ed49aa0c386608ce
child 458377 afa72a00cd2231e5c7293b3a2566cafc68bc0c99
push id77818
push usernbeleuzu@mozilla.com
push dateSat, 09 Feb 2019 03:44:14 +0000
treeherderautoland@fed534608adf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem, dveditz
bugs1524946
milestone67.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 1524946 - Ensure script private value stays alive during dynamic module import r=jandem a=dveditz
js/src/builtin/ModuleObject.cpp
js/src/vm/JSScript.cpp
js/src/vm/Runtime.h
--- a/js/src/builtin/ModuleObject.cpp
+++ b/js/src/builtin/ModuleObject.cpp
@@ -2,16 +2,17 @@
  * vim: set ts=8 sts=2 et sw=2 tw=80:
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "builtin/ModuleObject.h"
 
 #include "mozilla/EnumSet.h"
+#include "mozilla/ScopeExit.h"
 
 #include "builtin/Promise.h"
 #include "builtin/SelfHostingDefines.h"
 #include "frontend/ParseNode.h"
 #include "frontend/SharedContext.h"
 #include "gc/FreeOp.h"
 #include "gc/Policy.h"
 #include "gc/Tracer.h"
@@ -1674,16 +1675,20 @@ JSObject* js::CallModuleResolveHook(JSCo
 
   return result;
 }
 
 JSObject* js::StartDynamicModuleImport(JSContext* cx, HandleScript script,
                                        HandleValue specifierArg) {
   RootedValue referencingPrivate(cx,
                                  script->sourceObject()->canonicalPrivate());
+  cx->runtime()->addRefScriptPrivate(referencingPrivate);
+  auto releasePrivate = mozilla::MakeScopeExit([&] {
+    cx->runtime()->releaseScriptPrivate(referencingPrivate);
+  });
 
   RootedObject promiseConstructor(cx, JS::GetPromiseConstructor(cx));
   if (!promiseConstructor) {
     return nullptr;
   }
 
   RootedObject promiseObject(cx, JS::NewPromiseObject(cx, nullptr));
   if (!promiseObject) {
@@ -1715,25 +1720,30 @@ JSObject* js::StartDynamicModuleImport(J
     // anyway, so just return nullptr.
     if (!cx->isExceptionPending() ||
         !RejectPromiseWithPendingError(cx, promise)) {
       return nullptr;
     }
     return promise;
   }
 
+  releasePrivate.release();
   return promise;
 }
 
 bool js::FinishDynamicModuleImport(JSContext* cx,
                                    HandleValue referencingPrivate,
                                    HandleString specifier,
                                    HandleObject promiseArg) {
   Handle<PromiseObject*> promise = promiseArg.as<PromiseObject>();
 
+  auto releasePrivate = mozilla::MakeScopeExit([&] {
+    cx->runtime()->releaseScriptPrivate(referencingPrivate);
+  });
+
   if (cx->isExceptionPending()) {
     return RejectPromiseWithPendingError(cx, promise);
   }
 
   RootedObject result(cx,
                       CallModuleResolveHook(cx, referencingPrivate, specifier));
   if (!result) {
     return RejectPromiseWithPendingError(cx, promise);
--- a/js/src/vm/JSScript.cpp
+++ b/js/src/vm/JSScript.cpp
@@ -1470,27 +1470,19 @@ ScriptSourceObject* ScriptSourceObject::
 }
 
 void ScriptSourceObject::setPrivate(JSRuntime* rt, const Value& value) {
   // Update the private value, calling addRef/release hooks if necessary
   // to allow the embedding to maintain a reference count for the
   // private data.
   JS::AutoSuppressGCAnalysis nogc;
   Value prevValue = getReservedSlot(PRIVATE_SLOT);
-  if (!prevValue.isUndefined()) {
-    if (auto releaseHook = rt->scriptPrivateReleaseHook) {
-      releaseHook(prevValue);
-    }
-  }
+  rt->releaseScriptPrivate(prevValue);
   setReservedSlot(PRIVATE_SLOT, value);
-  if (!value.isUndefined()) {
-    if (auto addRefHook = rt->scriptPrivateAddRefHook) {
-      addRefHook(value);
-    }
-  }
+  rt->addRefScriptPrivate(value);
 }
 
 /* static */ bool JSScript::loadSource(JSContext* cx, ScriptSource* ss,
                                        bool* worked) {
   MOZ_ASSERT(!ss->hasSourceText());
   *worked = false;
   if (!cx->runtime()->sourceHook.ref() || !ss->sourceRetrievable()) {
     return true;
--- a/js/src/vm/Runtime.h
+++ b/js/src/vm/Runtime.h
@@ -967,16 +967,28 @@ struct JSRuntime : public js::MallocProv
   // HostImportModuleDynamically. This is also used to enable/disable dynamic
   // module import and can accessed by off-thread parsing.
   mozilla::Atomic<JS::ModuleDynamicImportHook> moduleDynamicImportHook;
 
   // Hooks called when script private references are created and destroyed.
   js::MainThreadData<JS::ScriptPrivateReferenceHook> scriptPrivateAddRefHook;
   js::MainThreadData<JS::ScriptPrivateReferenceHook> scriptPrivateReleaseHook;
 
+  void addRefScriptPrivate(const JS::Value& value) {
+    if (!value.isUndefined() && scriptPrivateAddRefHook) {
+      scriptPrivateAddRefHook(value);
+    }
+  }
+
+  void releaseScriptPrivate(const JS::Value& value) {
+    if (!value.isUndefined() && scriptPrivateReleaseHook) {
+      scriptPrivateReleaseHook(value);
+    }
+  }
+
  public:
 #if defined(JS_BUILD_BINAST)
   js::BinaryASTSupport& binast() { return binast_; }
 
  private:
   js::BinaryASTSupport binast_;
 #endif  // defined(JS_BUILD_BINAST)