Bug 1342012 - Support script and module private values which contain pointers to cycle-collected C++ objects r=jandem
authorJon Coppeard <jcoppeard@mozilla.com>
Thu, 06 Dec 2018 16:52:15 -0500
changeset 509499 2f619be4479861b2cf0781e8376fda79044920ef
parent 509498 93bc74cb72cfcf00b16eea08baa9187c3375e74d
child 509500 8e6f86cd811a93cce70fc91ac6fbb93405aff803
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1342012
milestone66.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 1342012 - Support script and module private values which contain pointers to cycle-collected C++ objects r=jandem
js/src/js.msg
js/src/jsapi.cpp
js/src/jsapi.h
js/src/jsfriendapi.cpp
js/src/jsfriendapi.h
js/src/vm/EnvironmentObject.cpp
js/src/vm/JSScript.cpp
js/src/vm/JSScript.h
js/src/vm/Runtime.cpp
js/src/vm/Runtime.h
xpcom/base/CycleCollectedJSRuntime.cpp
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -615,16 +615,17 @@ MSG_DEF(JSMSG_MISSING_INDIRECT_EXPORT,  
 MSG_DEF(JSMSG_AMBIGUOUS_INDIRECT_EXPORT, 0, JSEXN_SYNTAXERR, "ambiguous indirect export")
 MSG_DEF(JSMSG_MISSING_IMPORT,            0, JSEXN_SYNTAXERR, "import not found")
 MSG_DEF(JSMSG_AMBIGUOUS_IMPORT,          0, JSEXN_SYNTAXERR, "ambiguous import")
 MSG_DEF(JSMSG_MISSING_NAMESPACE_EXPORT,  0, JSEXN_SYNTAXERR, "export not found for namespace")
 MSG_DEF(JSMSG_MISSING_EXPORT,            1, JSEXN_SYNTAXERR, "local binding for export '{0}' not found")
 MSG_DEF(JSMSG_BAD_MODULE_STATUS,         0, JSEXN_INTERNALERR, "module record has unexpected status")
 MSG_DEF(JSMSG_NO_DYNAMIC_IMPORT,         0, JSEXN_SYNTAXERR, "dynamic module import is not implemented")
 MSG_DEF(JSMSG_IMPORT_SCRIPT_NOT_FOUND,   0, JSEXN_TYPEERR, "can't find referencing script for dynamic module import")
+MSG_DEF(JSMSG_BAD_MODULE_SPECIFIER,      1, JSEXN_TYPEERR, "error resolving module specifier '{0}'")
 
 // Promise
 MSG_DEF(JSMSG_CANNOT_RESOLVE_PROMISE_WITH_ITSELF,       0, JSEXN_TYPEERR, "A promise cannot be resolved with itself.")
 MSG_DEF(JSMSG_PROMISE_CAPABILITY_HAS_SOMETHING_ALREADY, 0, JSEXN_TYPEERR, "GetCapabilitiesExecutor function already invoked with non-undefined values.")
 MSG_DEF(JSMSG_PROMISE_RESOLVE_FUNCTION_NOT_CALLABLE,    0, JSEXN_TYPEERR, "A Promise subclass passed a non-callable value as the resolve function.")
 MSG_DEF(JSMSG_PROMISE_REJECT_FUNCTION_NOT_CALLABLE,     0, JSEXN_TYPEERR, "A Promise subclass passed a non-callable value as the reject function.")
 MSG_DEF(JSMSG_PROMISE_ERROR_IN_WRAPPED_REJECTION_REASON,0, JSEXN_INTERNALERR, "Promise rejection value is a non-unwrappable cross-compartment wrapper.")
 
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -3735,26 +3735,38 @@ JS_PUBLIC_API bool JS::CompileModule(JSC
 }
 
 JS_PUBLIC_API void JS::SetModulePrivate(JSObject* module,
                                         const JS::Value& value) {
   module->as<ModuleObject>().scriptSourceObject()->setPrivate(value);
 }
 
 JS_PUBLIC_API JS::Value JS::GetModulePrivate(JSObject* module) {
-  return module->as<ModuleObject>().scriptSourceObject()->unwrappedPrivate();
+  return module->as<ModuleObject>().scriptSourceObject()->canonicalPrivate();
 }
 
 JS_PUBLIC_API void JS::SetScriptPrivate(JSScript* script,
                                         const JS::Value& value) {
   script->sourceObject()->setPrivate(value);
 }
 
 JS_PUBLIC_API JS::Value JS::GetScriptPrivate(JSScript* script) {
-  return script->sourceObject()->unwrappedPrivate();
+  return script->sourceObject()->canonicalPrivate();
+}
+
+JS_PUBLIC_API JS::ScriptPrivateFinalizeHook JS::GetScriptPrivateFinalizeHook(
+    JSRuntime* rt) {
+  AssertHeapIsIdle();
+  return rt->scriptPrivateFinalizeHook;
+}
+
+JS_PUBLIC_API void JS::SetScriptPrivateFinalizeHook(
+    JSRuntime* rt, JS::ScriptPrivateFinalizeHook func) {
+  AssertHeapIsIdle();
+  rt->scriptPrivateFinalizeHook = func;
 }
 
 JS_PUBLIC_API bool JS::ModuleInstantiate(JSContext* cx,
                                          JS::HandleObject moduleArg) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
   cx->check(moduleArg);
   return ModuleObject::Instantiate(cx, moduleArg.as<ModuleObject>());
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -3099,23 +3099,27 @@ extern JS_PUBLIC_API void SetModuleMetad
                                                 ModuleMetadataHook func);
 
 using ModuleDynamicImportHook = bool (*)(JSContext* cx,
                                          HandleValue referencingPrivate,
                                          HandleString specifier,
                                          HandleObject promise);
 
 /**
- * Get the HostResolveImportedModule hook for the runtime.
+ * Get the HostImportModuleDynamically hook for the runtime.
  */
 extern JS_PUBLIC_API ModuleDynamicImportHook
 GetModuleDynamicImportHook(JSRuntime* rt);
 
 /**
- * Set the HostResolveImportedModule hook for the runtime to the given function.
+ * Set the HostImportModuleDynamically hook for the runtime to the given
+ * function.
+ *
+ * If this hook is not set (or set to nullptr) then the JS engine will throw an
+ * exception if dynamic module import is attempted.
  */
 extern JS_PUBLIC_API void SetModuleDynamicImportHook(
     JSRuntime* rt, ModuleDynamicImportHook func);
 
 extern JS_PUBLIC_API bool FinishDynamicModuleImport(
     JSContext* cx, HandleValue referencingPrivate, HandleString specifier,
     HandleObject promise);
 
@@ -3147,16 +3151,36 @@ extern JS_PUBLIC_API void SetScriptPriva
                                            const JS::Value& value);
 
 /**
  * Get the private value associated with a script. Note that this value is
  * shared by all nested scripts compiled from a single source file.
  */
 extern JS_PUBLIC_API JS::Value GetScriptPrivate(JSScript* script);
 
+/**
+ * A hook that's called whenever a script or module which has a private value
+ * set with SetScriptPrivate() or SetModulePrivate() is finalized. This can be
+ * used to clean up the private state. The private value is passed as an
+ * argument.
+ */
+using ScriptPrivateFinalizeHook = void (*)(JSFreeOp*, const JS::Value&);
+
+/**
+ * Get the script private finalize hook for the runtime.
+ */
+extern JS_PUBLIC_API ScriptPrivateFinalizeHook
+GetScriptPrivateFinalizeHook(JSRuntime* rt);
+
+/**
+ * Set the script private finalize hook for the runtime to the given function.
+ */
+extern JS_PUBLIC_API void SetScriptPrivateFinalizeHook(
+    JSRuntime* rt, ScriptPrivateFinalizeHook func);
+
 /*
  * Perform the ModuleInstantiate operation on the given source text module
  * record.
  *
  * This transitively resolves all module dependencies (calling the
  * HostResolveImportedModule hook) and initializes the environment record for
  * the module.
  */
--- a/js/src/jsfriendapi.cpp
+++ b/js/src/jsfriendapi.cpp
@@ -1399,11 +1399,19 @@ JS_FRIEND_API void js::LogCtor(void* sel
 }
 
 JS_FRIEND_API void js::LogDtor(void* self, const char* type, uint32_t sz) {
   if (LogCtorDtor fun = sLogDtor) {
     fun(self, type, sz);
   }
 }
 
+JS_FRIEND_API JS::Value js::MaybeGetScriptPrivate(JSObject* object) {
+  if (!object->is<ScriptSourceObject>()) {
+    return UndefinedValue();
+  }
+
+  return object->as<ScriptSourceObject>().canonicalPrivate();
+}
+
 JS_FRIEND_API uint64_t js::GetGCHeapUsageForObjectZone(JSObject* obj) {
   return obj->zone()->usage.gcBytes();
 }
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -101,16 +101,32 @@ extern JS_FRIEND_API bool JS_IsDeadWrapp
  * attempting to wrap objects from scopes which are already dead.
  *
  * If origObject is passed, it must be an proxy object, and will be
  * used to determine the characteristics of the new dead wrapper.
  */
 extern JS_FRIEND_API JSObject* JS_NewDeadWrapper(
     JSContext* cx, JSObject* origObject = nullptr);
 
+namespace js {
+
+/**
+ * Get the script private value associated with an object, if any.
+ *
+ * The private value is set with SetScriptPrivate() or SetModulePrivate() and is
+ * internally stored on the relevant ScriptSourceObject.
+ *
+ * This is used by the cycle collector to trace through
+ * ScriptSourceObjects. This allows private values to contain an nsISupports
+ * pointer and hence support references to cycle collected C++ objects.
+ */
+JS_FRIEND_API JS::Value MaybeGetScriptPrivate(JSObject* object);
+
+}  // namespace js
+
 /*
  * Used by the cycle collector to trace through a shape or object group and
  * all cycle-participating data it reaches, using bounded stack space.
  */
 extern JS_FRIEND_API void JS_TraceShapeCycleCollectorChildren(
     JS::CallbackTracer* trc, JS::GCCellPtr shape);
 extern JS_FRIEND_API void JS_TraceObjectGroupCycleCollectorChildren(
     JS::CallbackTracer* trc, JS::GCCellPtr group);
--- a/js/src/vm/EnvironmentObject.cpp
+++ b/js/src/vm/EnvironmentObject.cpp
@@ -3362,17 +3362,17 @@ ModuleObject* js::GetModuleObjectForScri
     }
   }
   return nullptr;
 }
 
 Value js::FindScriptOrModulePrivateForScript(JSScript* script) {
   while (script) {
     ScriptSourceObject* sso = script->sourceObject();
-    Value value = sso->unwrappedPrivate();
+    Value value = sso->canonicalPrivate();
     if (!value.isUndefined()) {
       return value;
     }
 
     MOZ_ASSERT(sso->unwrappedIntroductionScript() != script);
     script = sso->unwrappedIntroductionScript();
   }
 
--- a/js/src/vm/JSScript.cpp
+++ b/js/src/vm/JSScript.cpp
@@ -1312,16 +1312,26 @@ bool JSScript::hasScriptName() {
   auto p = realm()->scriptNameMap->lookup(this);
   return p.found();
 }
 
 void ScriptSourceObject::finalize(FreeOp* fop, JSObject* obj) {
   MOZ_ASSERT(fop->onMainThread());
   ScriptSourceObject* sso = &obj->as<ScriptSourceObject>();
   sso->source()->decref();
+
+  Value value = sso->canonicalPrivate();
+  if (!value.isUndefined()) {
+    // The embedding may need to dispose of its private data.
+    JS::AutoSuppressGCAnalysis suppressGC;
+    if (JS::ScriptPrivateFinalizeHook hook =
+            fop->runtime()->scriptPrivateFinalizeHook) {
+      hook(fop, value);
+    }
+  }
 }
 
 void ScriptSourceObject::trace(JSTracer* trc, JSObject* obj) {
   // This can be invoked during allocation of the SSO itself, before we've had a
   // chance to initialize things properly. In that case, there's nothing to
   // trace.
   if (obj->as<ScriptSourceObject>().hasSource()) {
     obj->as<ScriptSourceObject>().source()->trace(trc);
--- a/js/src/vm/JSScript.h
+++ b/js/src/vm/JSScript.h
@@ -1171,18 +1171,20 @@ class ScriptSourceObject : public Native
     return value.toGCThing()->as<JSScript>();
   }
 
   void setPrivate(const Value& value) {
     MOZ_ASSERT(isCanonical());
     setReservedSlot(PRIVATE_SLOT, value);
   }
 
-  Value unwrappedPrivate() const {
-    return unwrappedCanonical()->getReservedSlot(PRIVATE_SLOT);
+  Value canonicalPrivate() const {
+    Value value = getReservedSlot(PRIVATE_SLOT);
+    MOZ_ASSERT_IF(!isCanonical(), value.isUndefined());
+    return value;
   }
 
  private:
   enum {
     SOURCE_SLOT = 0,
     CANONICAL_SLOT,
     ELEMENT_SLOT,
     ELEMENT_PROPERTY_SLOT,
--- a/js/src/vm/Runtime.cpp
+++ b/js/src/vm/Runtime.cpp
@@ -158,17 +158,18 @@ JSRuntime::JSRuntime(JSRuntime* parentRu
       oomCallback(nullptr),
       debuggerMallocSizeOf(ReturnZeroSize),
       performanceMonitoring_(),
       stackFormat_(parentRuntime ? js::StackFormat::Default
                                  : js::StackFormat::SpiderMonkey),
       wasmInstances(mutexid::WasmRuntimeInstances),
       moduleResolveHook(),
       moduleMetadataHook(),
-      moduleDynamicImportHook() {
+      moduleDynamicImportHook(),
+      scriptPrivateFinalizeHook() {
   JS_COUNT_CTOR(JSRuntime);
   liveRuntimesCount++;
 
   // See function comment for why we call this now, not in JS_Init().
   wasm::EnsureEagerProcessSignalHandlers();
 }
 
 JSRuntime::~JSRuntime() {
--- a/js/src/vm/Runtime.h
+++ b/js/src/vm/Runtime.h
@@ -974,16 +974,19 @@ struct JSRuntime : public js::MallocProv
   // HostGetImportMetaProperties and HostFinalizeImportMeta.
   js::MainThreadData<JS::ModuleMetadataHook> moduleMetadataHook;
 
   // A hook that implements the abstract operation
   // HostImportModuleDynamically. This is also used to enable/disable dynamic
   // module import and can accessed by off-thread parsing.
   mozilla::Atomic<JS::ModuleDynamicImportHook> moduleDynamicImportHook;
 
+  // A hook called on script finalization.
+  js::MainThreadData<JS::ScriptPrivateFinalizeHook> scriptPrivateFinalizeHook;
+
  public:
 #if defined(JS_BUILD_BINAST)
   js::BinaryASTSupport& binast() { return binast_; }
 
  private:
   js::BinaryASTSupport binast_;
 #endif  // defined(JS_BUILD_BINAST)
 
--- a/xpcom/base/CycleCollectedJSRuntime.cpp
+++ b/xpcom/base/CycleCollectedJSRuntime.cpp
@@ -70,16 +70,17 @@
 #include "mozilla/dom/DOMJSClass.h"
 #include "mozilla/dom/ProfileTimelineMarkerBinding.h"
 #include "mozilla/dom/Promise.h"
 #include "mozilla/dom/PromiseBinding.h"
 #include "mozilla/dom/PromiseDebugging.h"
 #include "mozilla/dom/ScriptSettings.h"
 #include "js/Debug.h"
 #include "js/GCAPI.h"
+#include "jsfriendapi.h"
 #include "nsContentUtils.h"
 #include "nsCycleCollectionNoteRootCallback.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsCycleCollector.h"
 #include "nsDOMJSUtils.h"
 #include "nsExceptionHandler.h"
 #include "nsJSUtils.h"
 #include "nsWrapperCache.h"
@@ -622,38 +623,46 @@ void CycleCollectedJSRuntime::NoteGCThin
     nsCycleCollectionTraversalCallback& aCb) const {
   MOZ_ASSERT(aClasp);
   MOZ_ASSERT(aClasp == js::GetObjectClass(aObj));
 
   if (NoteCustomGCThingXPCOMChildren(aClasp, aObj, aCb)) {
     // Nothing else to do!
     return;
   }
+
   // XXX This test does seem fragile, we should probably whitelist classes
   //     that do hold a strong reference, but that might not be possible.
-  else if (aClasp->flags & JSCLASS_HAS_PRIVATE &&
-           aClasp->flags & JSCLASS_PRIVATE_IS_NSISUPPORTS) {
+  if (aClasp->flags & JSCLASS_HAS_PRIVATE &&
+      aClasp->flags & JSCLASS_PRIVATE_IS_NSISUPPORTS) {
     NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(aCb, "js::GetObjectPrivate(obj)");
     aCb.NoteXPCOMChild(static_cast<nsISupports*>(js::GetObjectPrivate(aObj)));
-  } else {
-    const DOMJSClass* domClass = GetDOMClass(aObj);
-    if (domClass) {
-      NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(aCb, "UnwrapDOMObject(obj)");
-      // It's possible that our object is an unforgeable holder object, in
-      // which case it doesn't actually have a C++ DOM object associated with
-      // it.  Use UnwrapPossiblyNotInitializedDOMObject, which produces null in
-      // that case, since NoteXPCOMChild/NoteNativeChild are null-safe.
-      if (domClass->mDOMObjectIsISupports) {
-        aCb.NoteXPCOMChild(
-            UnwrapPossiblyNotInitializedDOMObject<nsISupports>(aObj));
-      } else if (domClass->mParticipant) {
-        aCb.NoteNativeChild(UnwrapPossiblyNotInitializedDOMObject<void>(aObj),
-                            domClass->mParticipant);
-      }
+    return;
+  }
+
+  const DOMJSClass* domClass = GetDOMClass(aObj);
+  if (domClass) {
+    NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(aCb, "UnwrapDOMObject(obj)");
+    // It's possible that our object is an unforgeable holder object, in
+    // which case it doesn't actually have a C++ DOM object associated with
+    // it.  Use UnwrapPossiblyNotInitializedDOMObject, which produces null in
+    // that case, since NoteXPCOMChild/NoteNativeChild are null-safe.
+    if (domClass->mDOMObjectIsISupports) {
+      aCb.NoteXPCOMChild(
+          UnwrapPossiblyNotInitializedDOMObject<nsISupports>(aObj));
+    } else if (domClass->mParticipant) {
+      aCb.NoteNativeChild(UnwrapPossiblyNotInitializedDOMObject<void>(aObj),
+                          domClass->mParticipant);
     }
+    return;
+  }
+
+  JS::Value value = js::MaybeGetScriptPrivate(aObj);
+  if (!value.isUndefined()) {
+    aCb.NoteXPCOMChild(static_cast<nsISupports*>(value.toPrivate()));
   }
 }
 
 void CycleCollectedJSRuntime::TraverseGCThing(
     TraverseSelect aTs, JS::GCCellPtr aThing,
     nsCycleCollectionTraversalCallback& aCb) {
   bool isMarkedGray = JS::GCThingIsMarkedGray(aThing);