Index: source/gui/CGUI.cpp =================================================================== --- source/gui/CGUI.cpp +++ source/gui/CGUI.cpp @@ -1,4 +1,4 @@ -/* Copyright (C) 2022 Wildfire Games. +/* Copyright (C) 2023 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -129,7 +129,7 @@ ret = IN_HANDLED; ScriptRequest rq(m_ScriptInterface); - JS::RootedObject globalObj(rq.cx, rq.glob); + JS::RootedObject globalObj(rq.cx, &rq.globalObject()); JS::RootedValue result(rq.cx); if (!JS_CallFunctionValue(rq.cx, globalObj, m_GlobalHotkeys[hotkey][eventName], JS::HandleValueArray::empty(), &result)) ScriptException::CatchPending(rq); Index: source/gui/GUIManager.cpp =================================================================== --- source/gui/GUIManager.cpp +++ source/gui/GUIManager.cpp @@ -1,4 +1,4 @@ -/* Copyright (C) 2022 Wildfire Games. +/* Copyright (C) 2023 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -270,7 +270,7 @@ std::shared_ptr scriptInterface = gui->GetScriptInterface(); ScriptRequest rq(scriptInterface); - JS::RootedObject globalObj(rq.cx, rq.glob); + JS::RootedObject globalObj(rq.cx, &rq.globalObject()); JS::RootedValue funcVal(rq.cx, *callbackFunction); Index: source/gui/Scripting/JSInterface_GUIProxy_impl.h =================================================================== --- source/gui/Scripting/JSInterface_GUIProxy_impl.h +++ source/gui/Scripting/JSInterface_GUIProxy_impl.h @@ -165,7 +165,7 @@ template bool JSI_GUIProxy::get(JSContext* cx, JS::HandleObject proxy, JS::HandleValue UNUSED(receiver), JS::HandleId id, JS::MutableHandleValue vp) const { - ScriptRequest rq(cx); + ScriptRequest rq = ScriptRequest::FromAlreadyEntered(cx); T* e = IGUIProxyObject::FromPrivateSlot(proxy.get()); if (!e) @@ -242,7 +242,7 @@ return result.fail(JSMSG_OBJECT_REQUIRED); } - ScriptRequest rq(cx); + ScriptRequest rq = ScriptRequest::FromAlreadyEntered(cx); JS::RootedValue idval(rq.cx); if (!JS_IdToValue(rq.cx, id, &idval)) @@ -297,7 +297,7 @@ return result.fail(JSMSG_OBJECT_REQUIRED); } - ScriptRequest rq(cx); + ScriptRequest rq = ScriptRequest::FromAlreadyEntered(cx); JS::RootedValue idval(rq.cx); if (!JS_IdToValue(rq.cx, id, &idval)) Index: source/gui/Scripting/JSInterface_GUISize.cpp =================================================================== --- source/gui/Scripting/JSInterface_GUISize.cpp +++ source/gui/Scripting/JSInterface_GUISize.cpp @@ -1,4 +1,4 @@ -/* Copyright (C) 2021 Wildfire Games. +/* Copyright (C) 2023 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -48,8 +48,8 @@ bool JSI_GUISize::construct(JSContext* cx, uint argc, JS::Value* vp) { JS::CallArgs args = JS::CallArgsFromVp(argc, vp); - ScriptRequest rq(cx); - const ScriptInterface& scriptInterface = rq.GetScriptInterface(); + ScriptRequest rq = ScriptRequest::FromAlreadyEntered(cx); + const ScriptInterface& scriptInterface = rq.GetCurrentScriptInterface(); JS::RootedObject obj(rq.cx, scriptInterface.CreateCustomObject("GUISize")); @@ -107,7 +107,7 @@ JS::CallArgs args = JS::CallArgsFromVp(argc, vp); CStr buffer; - ScriptRequest rq(cx); + ScriptRequest rq = ScriptRequest::FromAlreadyEntered(cx); double val, valr; #define SIDE(side) \ Index: source/gui/SettingTypes/CGUISize.cpp =================================================================== --- source/gui/SettingTypes/CGUISize.cpp +++ source/gui/SettingTypes/CGUISize.cpp @@ -1,4 +1,4 @@ -/* Copyright (C) 2021 Wildfire Games. +/* Copyright (C) 2023 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -145,7 +145,7 @@ void CGUISize::ToJSVal(const ScriptRequest& rq, JS::MutableHandleValue ret) const { - const ScriptInterface& scriptInterface = rq.GetScriptInterface(); + const ScriptInterface& scriptInterface = rq.GetCurrentScriptInterface(); ret.setObjectOrNull(scriptInterface.CreateCustomObject("GUISize")); if (!ret.isObject()) Index: source/scriptinterface/FunctionWrapper.h =================================================================== --- source/scriptinterface/FunctionWrapper.h +++ source/scriptinterface/FunctionWrapper.h @@ -173,10 +173,10 @@ { // GCC (at least < 9) & VS17 prints warnings if arguments are not used in some constexpr branch. UNUSED2(rq); UNUSED2(args); UNUSED2(went_ok); - return std::forward_as_tuple(rq.GetScriptInterface()); + return std::forward_as_tuple(rq.GetCurrentScriptInterface()); } else - return std::tuple_cat(std::forward_as_tuple(rq.GetScriptInterface()), DoConvertFromJS<0, Types...>(rq, args, went_ok)); + return std::tuple_cat(std::forward_as_tuple(rq.GetCurrentScriptInterface()), DoConvertFromJS<0, Types...>(rq, args, went_ok)); } /////////////////////////////////////////////////////////////////////////// @@ -292,7 +292,7 @@ using ObjType = typename args_info::object_type; JS::CallArgs args = JS::CallArgsFromVp(argc, vp); - ScriptRequest rq(cx); + ScriptRequest rq = ScriptRequest::FromAlreadyEntered(cx); // If the callable is an object method, we must specify how to fetch the object. static_assert(std::is_same_v::object_type, void> || thisGetter != nullptr, @@ -387,7 +387,7 @@ template thisGetter = nullptr, u16 flags = JSPROP_ENUMERATE|JSPROP_READONLY|JSPROP_PERMANENT> static void Register(const ScriptRequest& rq, const char* name) { - JS_DefineFunction(rq.cx, rq.nativeScope, name, &ToJSNative, args_info::nb_args, flags); + JS_DefineFunction(rq.cx, rq.GetNativeScope(), name, &ToJSNative, args_info::nb_args, flags); } /** Index: source/scriptinterface/ScriptInterface.h =================================================================== --- source/scriptinterface/ScriptInterface.h +++ source/scriptinterface/ScriptInterface.h @@ -134,11 +134,16 @@ * entering the ScriptInterface compartment. It should only be used in specific situations, * for instance when initializing a persistent rooted. * If you need the compartmented context of the ScriptInterface, you should create a - * ScriptInterface::Request and use the context from that. + * ScriptRequest and use the context from that. */ JSContext* GetGeneralJSContext() const; std::shared_ptr GetContext() const; + /** + * Get the JS Object on which functions are placed into. + */ + JS::HandleObject GetNativeScope() const; + /** * Load global scripts that most script interfaces need, * located in the /globalscripts directory. VFS must be initialized. Index: source/scriptinterface/ScriptInterface.cpp =================================================================== --- source/scriptinterface/ScriptInterface.cpp +++ source/scriptinterface/ScriptInterface.cpp @@ -72,33 +72,48 @@ * Constructor for ScriptRequest - here because it needs access into ScriptInterface_impl. */ ScriptRequest::ScriptRequest(const ScriptInterface& scriptInterface) : - cx(scriptInterface.m->m_cx), - glob(scriptInterface.m->m_glob), - nativeScope(scriptInterface.m->m_nativeScope), - m_ScriptInterface(scriptInterface) + cx(scriptInterface.m->m_cx) { m_FormerRealm = JS::EnterRealm(cx, scriptInterface.m->m_glob); -} - -ScriptRequest::~ScriptRequest() -{ - JS::LeaveRealm(cx, m_FormerRealm); -} - -ScriptRequest::ScriptRequest(JSContext* cx) : ScriptRequest(ScriptInterface::CmptPrivate::GetScriptInterface(cx)) -{ + InitEnteredRealm(); } JS::Value ScriptRequest::globalValue() const { - return JS::ObjectValue(*glob); + CheckEnteredRealm(); + return JS::ObjectValue(*JS::GetRealmGlobalOrNull(JS::GetCurrentRealmOrNull(cx))); } -const ScriptInterface& ScriptRequest::GetScriptInterface() const +JSObject& ScriptRequest::globalObject() const { - return m_ScriptInterface; + CheckEnteredRealm(); + return *JS::GetRealmGlobalOrNull(JS::GetCurrentRealmOrNull(cx)); } +JS::HandleObject ScriptRequest::GetNativeScope() const +{ + CheckEnteredRealm(); + return GetCurrentScriptInterface().GetNativeScope(); +} + +const ScriptInterface& ScriptRequest::GetCurrentScriptInterface() const +{ + CheckEnteredRealm(); + return ScriptInterface::CmptPrivate::GetScriptInterface(cx); +} + +#if SCRIPT_REQUEST_CHECK_REALM +void ScriptRequest::InitEnteredRealm() +{ + m_EnteredRealm = JS::GetCurrentRealmOrNull(cx); +} + +void ScriptRequest::CheckEnteredRealm() const +{ + ENSURE(m_EnteredRealm == JS::GetCurrentRealmOrNull(cx)); +} +#endif + namespace { @@ -119,7 +134,7 @@ bool print(JSContext* cx, uint argc, JS::Value* vp) { JS::CallArgs args = JS::CallArgsFromVp(argc, vp); - ScriptRequest rq(cx); + ScriptRequest rq = ScriptRequest::FromAlreadyEntered(cx); for (uint i = 0; i < args.length(); ++i) { @@ -142,7 +157,7 @@ return true; } - ScriptRequest rq(cx); + ScriptRequest rq = ScriptRequest::FromAlreadyEntered(cx); std::wstring str; if (!Script::FromJSVal(rq, args[0], str)) return false; @@ -160,7 +175,7 @@ return true; } - ScriptRequest rq(cx); + ScriptRequest rq = ScriptRequest::FromAlreadyEntered(cx); std::wstring str; if (!Script::FromJSVal(rq, args[0], str)) return false; @@ -178,7 +193,7 @@ return true; } - ScriptRequest rq(cx); + ScriptRequest rq = ScriptRequest::FromAlreadyEntered(cx); std::wstring str; if (!Script::FromJSVal(rq, args[0], str)) return false; @@ -358,7 +373,7 @@ ScriptRequest rq(this); m_CmptPrivate.pScriptInterface = this; - JS::SetRealmPrivate(JS::GetObjectRealmOrNull(rq.glob), (void*)&m_CmptPrivate); + JS::SetRealmPrivate(JS::GetObjectRealmOrNull(&rq.globalObject()), (void*)&m_CmptPrivate); } ScriptInterface::ScriptInterface(const char* nativeScopeName, const char* debugName, const ScriptInterface& neighbor) @@ -376,7 +391,7 @@ ScriptRequest rq(this); m_CmptPrivate.pScriptInterface = this; - JS::SetRealmPrivate(JS::GetObjectRealmOrNull(rq.glob), (void*)&m_CmptPrivate); + JS::SetRealmPrivate(JS::GetObjectRealmOrNull(&rq.globalObject()), (void*)&m_CmptPrivate); } ScriptInterface::~ScriptInterface() @@ -435,7 +450,7 @@ { ScriptRequest rq(this); JS::RootedValue math(rq.cx); - JS::RootedObject global(rq.cx, rq.glob); + JS::RootedObject global(rq.cx, &rq.globalObject()); if (JS_GetProperty(rq.cx, global, "Math", &math) && math.isObject()) { JS::RootedObject mathObj(rq.cx, &math.toObject()); @@ -463,6 +478,11 @@ return m->m_context; } +JS::HandleObject ScriptInterface::GetNativeScope() const +{ + return m->m_nativeScope; +} + void ScriptInterface::CallConstructor(JS::HandleValue ctor, JS::HandleValueArray argv, JS::MutableHandleValue out) const { ScriptRequest rq(this); @@ -491,7 +511,7 @@ throw PSERROR_Scripting_DefineType_AlreadyExists(); } - JS::RootedObject global(rq.cx, rq.glob); + JS::RootedObject global(rq.cx, &rq.globalObject()); JS::RootedObject obj(rq.cx, JS_InitClass(rq.cx, global, nullptr, clasp, constructor, minArgs, // Constructor, min args @@ -526,7 +546,7 @@ bool ScriptInterface::SetGlobal_(const char* name, JS::HandleValue value, bool replace, bool constant, bool enumerate) { ScriptRequest rq(this); - JS::RootedObject global(rq.cx, rq.glob); + JS::RootedObject global(rq.cx, &rq.globalObject()); bool found; if (!JS_HasProperty(rq.cx, global, name, &found)) @@ -570,7 +590,7 @@ bool ScriptInterface::GetGlobalProperty(const ScriptRequest& rq, const std::string& name, JS::MutableHandleValue out) { // Try to get the object as a property of the global object. - JS::RootedObject global(rq.cx, rq.glob); + JS::RootedObject global(rq.cx, &rq.globalObject()); if (!JS_GetProperty(rq.cx, global, name.c_str(), out)) { out.set(JS::NullHandleValue); @@ -583,7 +603,7 @@ // Some objects, such as const definitions, or Class definitions, are hidden inside closures. // We must fetch those from the correct lexical scope. //JS::RootedValue glob(cx); - JS::RootedObject lexical_environment(rq.cx, JS_GlobalLexicalEnvironment(rq.glob)); + JS::RootedObject lexical_environment(rq.cx, JS_GlobalLexicalEnvironment(&rq.globalObject())); if (!JS_GetProperty(rq.cx, lexical_environment, name.c_str(), out)) { out.set(JS::NullHandleValue); @@ -610,7 +630,7 @@ bool ScriptInterface::LoadScript(const VfsPath& filename, const std::string& code) const { ScriptRequest rq(this); - JS::RootedObject global(rq.cx, rq.glob); + JS::RootedObject global(rq.cx, &rq.globalObject()); // CompileOptions does not copy the contents of the filename string pointer. // Passing a temporary string there will cause undefined behaviour, so we create a separate string to avoid the temporary. Index: source/scriptinterface/ScriptRequest.h =================================================================== --- source/scriptinterface/ScriptRequest.h +++ source/scriptinterface/ScriptRequest.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2021 Wildfire Games. +/* Copyright (C) 2023 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -39,6 +39,12 @@ #include +#ifdef NDEBUG +#define SCRIPT_REQUEST_CHECK_REALM 0 +#else +#define SCRIPT_REQUEST_CHECK_REALM 1 +#endif + class ScriptInterface; /** @@ -69,33 +75,76 @@ ScriptRequest(const ScriptInterface& scriptInterface); ScriptRequest(const ScriptInterface* scriptInterface) : ScriptRequest(*scriptInterface) {} ScriptRequest(std::shared_ptr scriptInterface) : ScriptRequest(*scriptInterface) {} - ~ScriptRequest(); + + ~ScriptRequest() { + if (m_FormerRealm != reinterpret_cast(this)) + JS::LeaveRealm(cx, m_FormerRealm); + } /** - * Create a script request from a JSContext. - * This can be used to get the script interface in a JSNative function. - * In general, you shouldn't have to rely on this otherwise. + * Return the scriptInterface of the currently entered realm. + * NB: this will not return the 'expected' ScriptInterface if another realm was entered + * since this particular ScriptRequest object was created. + * If that behaviour is needed, pass ScriptInterface objects instead. + * Since that is almost certainly a code bug, this is asserted in debug builds. + * This is mostly a convenience to avoid passing both a request and a scriptInterface. */ - ScriptRequest(JSContext* cx); + const ScriptInterface& GetCurrentScriptInterface() const; /** - * Return the scriptInterface active when creating this ScriptRequest. - * Note that this is multi-request safe: even if another ScriptRequest is created, - * it will point to the original scriptInterface, and thus can be used to re-enter the realm. + * Return the global object of the current context as a value (aka Realm aka ScriptInterface). + * Goes through ScriptInterface - don't use in hot code. */ - const ScriptInterface& GetScriptInterface() const; - JS::Value globalValue() const; + /** + * Return the global object of the current context (aka Realm aka ScriptInterface). + * Goes through ScriptInterface - don't use in hot code. + */ + JSObject& globalObject() const; + + /** + * Return the object on which functions are exposed. + * Goes through ScriptInterface - don't use in hot code. + * Primarily exists to avoid including ScriptInterface in FunctionWrapper.h + */ + JS::HandleObject GetNativeScope() const; + + /** + * Create a script request from a JSContext. + * This can be used to get the script interface in e.g. a JSNative function. + * As the name implies, this does not enter any realm, so we must already be in the correct one. + */ + static ScriptRequest FromAlreadyEntered(JSContext* cx) { + return ScriptRequest(cx); + } + // Note that JSContext actually changes behind the scenes when creating another ScriptRequest for another realm, // so be _very_ careful when juggling between different realms. JSContext* cx; - JS::HandleObject glob; - JS::HandleObject nativeScope; + private: - const ScriptInterface& m_ScriptInterface; + // @see fromAlreadyEntered + // Store the address of the ScriptRequest object in m_FormerRealm as a safe tagged value, + // indicating that we didn't actually enter anything. + // This value is safe as it can't conflict with an actual JS::Realm. + ScriptRequest(JSContext* cx) : cx(cx), m_FormerRealm(reinterpret_cast(this)) { + InitEnteredRealm(); + } + +#if SCRIPT_REQUEST_CHECK_REALM + /** + * In debug mode, ensures that scriptRequests are used correctly. + */ + void InitEnteredRealm(); + void CheckEnteredRealm() const; + + JS::Realm* m_EnteredRealm = nullptr; +#else + void InitEnteredRealm() {}; + void CheckEnteredRealm() const {}; +#endif JS::Realm* m_FormerRealm; }; - #endif // INCLUDED_SCRIPTREQUEST Index: source/simulation2/components/CCmpAIManager.cpp =================================================================== --- source/simulation2/components/CCmpAIManager.cpp +++ source/simulation2/components/CCmpAIManager.cpp @@ -1,4 +1,4 @@ -/* Copyright (C) 2022 Wildfire Games. +/* Copyright (C) 2023 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -252,7 +252,7 @@ // Register the sim globals for easy & explicit access. Mark it replaceable for hotloading. JS::RootedValue global(rq.cx, simrq.globalValue()); m_ScriptInterface->SetGlobal("Sim", global, true); - JS::RootedValue scope(rq.cx, JS::ObjectValue(*simrq.nativeScope.get())); + JS::RootedValue scope(rq.cx, JS::ObjectValue(*simrq.GetNativeScope().get())); m_ScriptInterface->SetGlobal("SimEngine", scope, true); } Index: source/simulation2/scripting/EngineScriptConversions.cpp =================================================================== --- source/simulation2/scripting/EngineScriptConversions.cpp +++ source/simulation2/scripting/EngineScriptConversions.cpp @@ -54,7 +54,7 @@ // Otherwise we need to construct a wrapper object // (TODO: cache wrapper objects?) JS::RootedObject obj(rq.cx); - if (!val->NewJSObject(rq.GetScriptInterface(), &obj)) + if (!val->NewJSObject(rq.GetCurrentScriptInterface(), &obj)) { // Report as an error, since scripts really shouldn't try to use unscriptable interfaces LOGERROR("IComponent does not have a scriptable interface"); @@ -156,7 +156,7 @@ template<> void Script::ToJSVal(const ScriptRequest& rq, JS::MutableHandleValue ret, const CFixedVector3D& val) { - JS::RootedObject global(rq.cx, rq.glob); + JS::RootedObject global(rq.cx, &rq.globalObject()); JS::RootedValue valueVector3D(rq.cx); if (!ScriptInterface::GetGlobalProperty(rq, "Vector3D", &valueVector3D)) FAIL_VOID("Failed to get Vector3D constructor"); @@ -192,7 +192,7 @@ template<> void Script::ToJSVal(const ScriptRequest& rq, JS::MutableHandleValue ret, const CFixedVector2D& val) { - JS::RootedObject global(rq.cx, rq.glob); + JS::RootedObject global(rq.cx, &rq.globalObject()); JS::RootedValue valueVector2D(rq.cx); if (!ScriptInterface::GetGlobalProperty(rq, "Vector2D", &valueVector2D)) FAIL_VOID("Failed to get Vector2D constructor");