Bug 1524946 - Ensure script private value stays alive during dynamic module import. r=jandem, a=dveditz
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 08 Feb 2019 18:39:54 +0000
changeset 512985 2305c930b771669f33078d7c85d404e63b6f3e81
parent 512984 f05abb753b64609a9e169a08c66fc237615b3b50
child 512986 475bc76114c22418684c4cb12c9fc0ec02db79b1
push id10669
push userryanvm@gmail.com
push dateMon, 11 Feb 2019 12:53:17 +0000
treeherdermozilla-beta@2305c930b771 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem, dveditz
bugs1524946
milestone66.0
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"
@@ -1675,19 +1676,16 @@ JSObject* js::CallModuleResolveHook(JSCo
     return nullptr;
   }
 
   return result;
 }
 
 JSObject* js::StartDynamicModuleImport(JSContext* cx, HandleScript script,
                                        HandleValue specifierArg) {
-  RootedValue referencingPrivate(cx,
-                                 script->sourceObject()->canonicalPrivate());
-
   RootedObject promiseConstructor(cx, JS::GetPromiseConstructor(cx));
   if (!promiseConstructor) {
     return nullptr;
   }
 
   RootedObject promiseObject(cx, JS::NewPromiseObject(cx, nullptr));
   if (!promiseObject) {
     return nullptr;
@@ -1708,17 +1706,23 @@ JSObject* js::StartDynamicModuleImport(J
   RootedString specifier(cx, ToString(cx, specifierArg));
   if (!specifier) {
     if (!RejectPromiseWithPendingError(cx, promise)) {
       return nullptr;
     }
     return promise;
   }
 
+  RootedValue referencingPrivate(cx,
+                                 script->sourceObject()->canonicalPrivate());
+  cx->runtime()->addRefScriptPrivate(referencingPrivate);
+
   if (!importHook(cx, referencingPrivate, specifier, promise)) {
+    cx->runtime()->releaseScriptPrivate(referencingPrivate);
+
     // If there's no exception pending then the script is terminating
     // anyway, so just return nullptr.
     if (!cx->isExceptionPending() ||
         !RejectPromiseWithPendingError(cx, promise)) {
       return nullptr;
     }
     return promise;
   }
@@ -1727,16 +1731,20 @@ JSObject* js::StartDynamicModuleImport(J
 }
 
 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
@@ -1471,27 +1471,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)