Wrap JSAutoRequest and replace usage of JSContext* with the wrapper.

JSAutoRequest is required before calling into most JSAPI methods, for GC
reasons.
Calling it is required and fragile as one must not forget.
Further, SM52 and later make manipulating JSContext* dangerous as that
can cross Compartment(Realm in SM68) barriers (and ScriptInterface now
matches a Compartment).

The solution to both problems is to avoid using JSContext* in 0 A.D.
itself. To achieve this, a Request class is introduced, and must be used
to access a JSContext* from a scriptInterface. Further, Request is
passed to other ScriptInterface functions isntead of JSContext*, making
it obvious that the caller has already called it, reducing errors and
redundant JSAutoRequest calls.
Only JSNative functions now get a naked JSContext* without protection,
but the likelihood of forgetting a request is lower since many
ScriptInterface functions now expect it.

JSContext* is directly passed to JSAPI functions only.

Part of the SM52 migration, stage: SM45 compatible

Based on a patch by: Itms
Tested By: Freagarach
Refs #4893

Differential Revision: https://code.wildfiregames.com/D3088
This was SVN commit r24176.
This commit is contained in:
wraitii
2020-11-13 13:18:22 +00:00
parent 6a029d2a84
commit ee0d204bf6
74 changed files with 1591 additions and 1812 deletions
@@ -1,4 +1,4 @@
/* Copyright (C) 2017 Wildfire Games.
/* Copyright (C) 2020 Wildfire Games.
* This file is part of 0 A.D.
*
* 0 A.D. is free software: you can redistribute it and/or modify
@@ -26,8 +26,7 @@ CComponentTypeScript::CComponentTypeScript(const ScriptInterface& scriptInterfac
m_ScriptInterface(scriptInterface), m_Instance(scriptInterface.GetJSRuntime(), instance)
{
// Cache the property detection for efficiency
JSContext* cx = m_ScriptInterface.GetContext();
JSAutoRequest rq(cx);
ScriptInterface::Request rq(m_ScriptInterface);
m_HasCustomSerialize = m_ScriptInterface.HasProperty(m_Instance, "Serialize");
m_HasCustomDeserialize = m_ScriptInterface.HasProperty(m_Instance, "Deserialize");
@@ -35,7 +34,7 @@ CComponentTypeScript::CComponentTypeScript(const ScriptInterface& scriptInterfac
m_HasNullSerialize = false;
if (m_HasCustomSerialize)
{
JS::RootedValue val(cx);
JS::RootedValue val(rq.cx);
if (m_ScriptInterface.GetProperty(m_Instance, "Serialize", &val) && val.isNull())
m_HasNullSerialize = true;
}
@@ -55,12 +54,11 @@ void CComponentTypeScript::Deinit()
void CComponentTypeScript::HandleMessage(const CMessage& msg, bool global)
{
JSContext* cx = m_ScriptInterface.GetContext();
JSAutoRequest rq(cx);
ScriptInterface::Request rq(m_ScriptInterface);
const char* name = global ? msg.GetScriptGlobalHandlerName() : msg.GetScriptHandlerName();
JS::RootedValue msgVal(cx, msg.ToJSValCached(m_ScriptInterface));
JS::RootedValue msgVal(rq.cx, msg.ToJSValCached(m_ScriptInterface));
if (!m_ScriptInterface.CallFunctionVoid(m_Instance, name, msgVal))
LOGERROR("Script message handler %s failed", name);
@@ -72,14 +70,13 @@ void CComponentTypeScript::Serialize(ISerializer& serialize)
if (m_HasNullSerialize)
return;
JSContext* cx = m_ScriptInterface.GetContext();
JSAutoRequest rq(cx);
ScriptInterface::Request rq(m_ScriptInterface);
// Support a custom "Serialize" function, which returns a new object that will be
// serialized instead of the component itself
if (m_HasCustomSerialize)
{
JS::RootedValue val(cx);
JS::RootedValue val(rq.cx);
if (!m_ScriptInterface.CallFunction(m_Instance, "Serialize", &val))
LOGERROR("Script Serialize call failed");
serialize.ScriptVal("object", &val);
@@ -92,8 +89,7 @@ void CComponentTypeScript::Serialize(ISerializer& serialize)
void CComponentTypeScript::Deserialize(const CParamNode& paramNode, IDeserializer& deserialize, entity_id_t ent)
{
JSContext* cx = m_ScriptInterface.GetContext();
JSAutoRequest rq(cx);
ScriptInterface::Request rq(m_ScriptInterface);
m_ScriptInterface.SetProperty(m_Instance, "entity", (int)ent, true, false);
m_ScriptInterface.SetProperty(m_Instance, "template", paramNode, true, false);
@@ -102,7 +98,7 @@ void CComponentTypeScript::Deserialize(const CParamNode& paramNode, IDeserialize
// instead of automatically adding the deserialized properties onto the object
if (m_HasCustomDeserialize)
{
JS::RootedValue val(cx);
JS::RootedValue val(rq.cx);
// If Serialize = null, we'll still call Deserialize but with undefined argument
if (!m_HasNullSerialize)