From 20a91427ef547ded9c27f72ae20e7c970c1719f6 Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Thu, 6 Dec 2018 16:52:15 -0500 Subject: [PATCH] Bug 1342012 - Support script and module private values which contain pointers to cycle-collected C++ objects r=jandem --- js/src/js.msg | 1 + js/src/jsapi.cpp | 16 ++++++++-- js/src/jsapi.h | 28 +++++++++++++++-- js/src/jsfriendapi.cpp | 8 +++++ js/src/jsfriendapi.h | 16 ++++++++++ js/src/vm/EnvironmentObject.cpp | 2 +- js/src/vm/JSScript.cpp | 10 ++++++ js/src/vm/JSScript.h | 6 ++-- js/src/vm/Runtime.cpp | 3 +- js/src/vm/Runtime.h | 3 ++ xpcom/base/CycleCollectedJSRuntime.cpp | 43 ++++++++++++++++---------- 11 files changed, 111 insertions(+), 25 deletions(-) diff --git a/js/src/js.msg b/js/src/js.msg index b4e0a680e5cf..44930a5c4b73 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -620,6 +620,7 @@ MSG_DEF(JSMSG_MISSING_EXPORT, 1, JSEXN_SYNTAXERR, "local binding for 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.") diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 4928f6c3b992..a58f95d5fc54 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -3740,7 +3740,7 @@ JS_PUBLIC_API void JS::SetModulePrivate(JSObject* module, } JS_PUBLIC_API JS::Value JS::GetModulePrivate(JSObject* module) { - return module->as().scriptSourceObject()->unwrappedPrivate(); + return module->as().scriptSourceObject()->canonicalPrivate(); } JS_PUBLIC_API void JS::SetScriptPrivate(JSScript* script, @@ -3749,7 +3749,19 @@ JS_PUBLIC_API void JS::SetScriptPrivate(JSScript* script, } 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, diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 26fb257374c2..03ec54972eb4 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -3104,13 +3104,17 @@ using ModuleDynamicImportHook = bool (*)(JSContext* cx, 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); @@ -3152,6 +3156,26 @@ extern JS_PUBLIC_API void SetScriptPrivate(JSScript* script, */ 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. diff --git a/js/src/jsfriendapi.cpp b/js/src/jsfriendapi.cpp index 33a68b951fe6..0ce63d9e98ba 100644 --- a/js/src/jsfriendapi.cpp +++ b/js/src/jsfriendapi.cpp @@ -1404,6 +1404,14 @@ JS_FRIEND_API void js::LogDtor(void* self, const char* type, uint32_t sz) { } } +JS_FRIEND_API JS::Value js::MaybeGetScriptPrivate(JSObject* object) { + if (!object->is()) { + return UndefinedValue(); + } + + return object->as().canonicalPrivate(); +} + JS_FRIEND_API uint64_t js::GetGCHeapUsageForObjectZone(JSObject* obj) { return obj->zone()->usage.gcBytes(); } diff --git a/js/src/jsfriendapi.h b/js/src/jsfriendapi.h index b0fb8ea43989..bee1523517c9 100644 --- a/js/src/jsfriendapi.h +++ b/js/src/jsfriendapi.h @@ -106,6 +106,22 @@ extern JS_FRIEND_API bool JS_IsDeadWrapper(JSObject* obj); 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. diff --git a/js/src/vm/EnvironmentObject.cpp b/js/src/vm/EnvironmentObject.cpp index 0c693b44fb5f..d492bff90523 100644 --- a/js/src/vm/EnvironmentObject.cpp +++ b/js/src/vm/EnvironmentObject.cpp @@ -3367,7 +3367,7 @@ ModuleObject* js::GetModuleObjectForScript(JSScript* script) { Value js::FindScriptOrModulePrivateForScript(JSScript* script) { while (script) { ScriptSourceObject* sso = script->sourceObject(); - Value value = sso->unwrappedPrivate(); + Value value = sso->canonicalPrivate(); if (!value.isUndefined()) { return value; } diff --git a/js/src/vm/JSScript.cpp b/js/src/vm/JSScript.cpp index d6cc988beaa7..fed9b466739e 100644 --- a/js/src/vm/JSScript.cpp +++ b/js/src/vm/JSScript.cpp @@ -1317,6 +1317,16 @@ void ScriptSourceObject::finalize(FreeOp* fop, JSObject* obj) { MOZ_ASSERT(fop->onMainThread()); ScriptSourceObject* sso = &obj->as(); 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) { diff --git a/js/src/vm/JSScript.h b/js/src/vm/JSScript.h index c95263ee851e..06a3ff359ae0 100644 --- a/js/src/vm/JSScript.h +++ b/js/src/vm/JSScript.h @@ -1176,8 +1176,10 @@ class ScriptSourceObject : public NativeObject { 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: diff --git a/js/src/vm/Runtime.cpp b/js/src/vm/Runtime.cpp index 26ddfe5e4bd8..b3780d0dfae1 100644 --- a/js/src/vm/Runtime.cpp +++ b/js/src/vm/Runtime.cpp @@ -163,7 +163,8 @@ JSRuntime::JSRuntime(JSRuntime* parentRuntime) wasmInstances(mutexid::WasmRuntimeInstances), moduleResolveHook(), moduleMetadataHook(), - moduleDynamicImportHook() { + moduleDynamicImportHook(), + scriptPrivateFinalizeHook() { JS_COUNT_CTOR(JSRuntime); liveRuntimesCount++; diff --git a/js/src/vm/Runtime.h b/js/src/vm/Runtime.h index 66bc001ab3e8..03758a54c54e 100644 --- a/js/src/vm/Runtime.h +++ b/js/src/vm/Runtime.h @@ -979,6 +979,9 @@ struct JSRuntime : public js::MallocProvider { // module import and can accessed by off-thread parsing. mozilla::Atomic moduleDynamicImportHook; + // A hook called on script finalization. + js::MainThreadData scriptPrivateFinalizeHook; + public: #if defined(JS_BUILD_BINAST) js::BinaryASTSupport& binast() { return binast_; } diff --git a/xpcom/base/CycleCollectedJSRuntime.cpp b/xpcom/base/CycleCollectedJSRuntime.cpp index 5a7585c89bb0..901930444eda 100644 --- a/xpcom/base/CycleCollectedJSRuntime.cpp +++ b/xpcom/base/CycleCollectedJSRuntime.cpp @@ -75,6 +75,7 @@ #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" @@ -627,28 +628,36 @@ void CycleCollectedJSRuntime::NoteGCThingXPCOMChildren( // 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(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(aObj)); - } else if (domClass->mParticipant) { - aCb.NoteNativeChild(UnwrapPossiblyNotInitializedDOMObject(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(aObj)); + } else if (domClass->mParticipant) { + aCb.NoteNativeChild(UnwrapPossiblyNotInitializedDOMObject(aObj), + domClass->mParticipant); } + return; + } + + JS::Value value = js::MaybeGetScriptPrivate(aObj); + if (!value.isUndefined()) { + aCb.NoteXPCOMChild(static_cast(value.toPrivate())); } }