Index: ps/trunk/libraries/osx/build-osx-libs.sh =================================================================== --- ps/trunk/libraries/osx/build-osx-libs.sh +++ ps/trunk/libraries/osx/build-osx-libs.sh @@ -53,7 +53,7 @@ # * FCollada # -------------------------------------------------------------- # We use suffixes here in order to force rebuilding when patching these libs -SPIDERMONKEY_VERSION="mozjs-45.0.2+wildfiregames.1" +SPIDERMONKEY_VERSION="mozjs-45.0.2+wildfiregames.2" NVTT_VERSION="nvtt-2.1.1+wildfiregames.1" FCOLLADA_VERSION="fcollada-3.05+wildfiregames.1" # -------------------------------------------------------------- Index: ps/trunk/libraries/source/spidermonkey/FixPersistentRootedSymbol.diff =================================================================== --- ps/trunk/libraries/source/spidermonkey/FixPersistentRootedSymbol.diff +++ ps/trunk/libraries/source/spidermonkey/FixPersistentRootedSymbol.diff @@ -0,0 +1,13 @@ +diff --git a/js/public/RootingAPI.h b/js/public/RootingAPI.h +index 4e925cba04..06d2a95440 100644 +--- a/js/public/RootingAPI.h ++++ b/js/public/RootingAPI.h +@@ -1019,6 +1019,7 @@ class PersistentRooted : public js::PersistentRootedBase, + MOZ_ASSERT(kind == js::THING_ROOT_OBJECT || + kind == js::THING_ROOT_SCRIPT || + kind == js::THING_ROOT_STRING || ++ kind == js::THING_ROOT_SYMBOL || + kind == js::THING_ROOT_ID || + kind == js::THING_ROOT_VALUE || + kind == js::THING_ROOT_TRACEABLE); + Index: ps/trunk/libraries/source/spidermonkey/include-win32-debug/js/RootingAPI.h =================================================================== --- ps/trunk/libraries/source/spidermonkey/include-win32-debug/js/RootingAPI.h +++ ps/trunk/libraries/source/spidermonkey/include-win32-debug/js/RootingAPI.h @@ -1019,6 +1019,7 @@ MOZ_ASSERT(kind == js::THING_ROOT_OBJECT || kind == js::THING_ROOT_SCRIPT || kind == js::THING_ROOT_STRING || + kind == js::THING_ROOT_SYMBOL || kind == js::THING_ROOT_ID || kind == js::THING_ROOT_VALUE || kind == js::THING_ROOT_TRACEABLE); Index: ps/trunk/libraries/source/spidermonkey/include-win32-release/js/RootingAPI.h =================================================================== --- ps/trunk/libraries/source/spidermonkey/include-win32-release/js/RootingAPI.h +++ ps/trunk/libraries/source/spidermonkey/include-win32-release/js/RootingAPI.h @@ -1019,6 +1019,7 @@ MOZ_ASSERT(kind == js::THING_ROOT_OBJECT || kind == js::THING_ROOT_SCRIPT || kind == js::THING_ROOT_STRING || + kind == js::THING_ROOT_SYMBOL || kind == js::THING_ROOT_ID || kind == js::THING_ROOT_VALUE || kind == js::THING_ROOT_TRACEABLE); Index: ps/trunk/libraries/source/spidermonkey/patch.sh =================================================================== --- ps/trunk/libraries/source/spidermonkey/patch.sh +++ ps/trunk/libraries/source/spidermonkey/patch.sh @@ -54,3 +54,7 @@ # Will be included in SM52. # https://bugzilla.mozilla.org/show_bug.cgi?id=1379538 patch -p1 < ../FixLinking.diff + +# Allow the use of JS::PersistentRootedSymbol in debug builds. +# This check will be removed in SM52. +patch -p1 < ../FixPersistentRootedSymbol.diff Index: ps/trunk/source/scriptinterface/ScriptRuntime.h =================================================================== --- ps/trunk/source/scriptinterface/ScriptRuntime.h +++ ps/trunk/source/scriptinterface/ScriptRuntime.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2018 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 @@ -57,32 +57,18 @@ void RegisterContext(JSContext* cx); void UnRegisterContext(JSContext* cx); - /** - * Registers an object to be freed/finalized by the ScriptRuntime. Freeing is - * guaranteed to happen after the next minor GC has completed, but might also - * happen a bit later. This is only needed in very special situations - * and you should only use it if you know exactly why you need it! - * Specify a deleter for the shared_ptr to free the void pointer correctly - * (by casting to the right type before calling delete for example). - */ - void AddDeferredFinalizationObject(const std::shared_ptr& obj); - JSRuntime* m_rt; private: void PrepareContextsForIncrementalGC(); - void GCCallbackMember(); std::list m_Contexts; - std::vector > m_FinalizationListObjectIdCache; int m_RuntimeSize; int m_HeapGrowthBytesGCTrigger; int m_LastGCBytes; double m_LastGCCheck; - - static void GCCallback(JSRuntime *rt, JSGCStatus status, void *data); }; #endif // INCLUDED_SCRIPTRUNTIME Index: ps/trunk/source/scriptinterface/ScriptRuntime.cpp =================================================================== --- ps/trunk/source/scriptinterface/ScriptRuntime.cpp +++ ps/trunk/source/scriptinterface/ScriptRuntime.cpp @@ -1,4 +1,4 @@ -/* Copyright (C) 2016 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 @@ -89,22 +89,6 @@ #endif } -void ScriptRuntime::GCCallback(JSRuntime* UNUSED(rt), JSGCStatus status, void *data) -{ - if (status == JSGC_END) - reinterpret_cast(data)->GCCallbackMember(); -} - -void ScriptRuntime::GCCallbackMember() -{ - m_FinalizationListObjectIdCache.clear(); -} - -void ScriptRuntime::AddDeferredFinalizationObject(const std::shared_ptr& obj) -{ - m_FinalizationListObjectIdCache.push_back(obj); -} - ScriptRuntime::ScriptRuntime(shared_ptr parentRuntime, int runtimeSize, int heapGrowthBytesGCTrigger): m_LastGCBytes(0), m_LastGCCheck(0.0f), @@ -118,7 +102,6 @@ ENSURE(m_rt); // TODO: error handling JS::SetGCSliceCallback(m_rt, GCSliceCallbackHook); - JS_SetGCCallback(m_rt, ScriptRuntime::GCCallback, this); JS_SetGCParameter(m_rt, JSGC_MAX_MALLOC_BYTES, m_RuntimeSize); JS_SetGCParameter(m_rt, JSGC_MAX_BYTES, m_RuntimeSize); @@ -133,9 +116,7 @@ ScriptRuntime::~ScriptRuntime() { - JS_SetGCCallback(m_rt, nullptr, nullptr); JS_DestroyRuntime(m_rt); - ENSURE(m_FinalizationListObjectIdCache.empty() && "Leak: Removing callback while some objects still aren't finalized!"); ENSURE(ScriptEngine::IsInitialised() && "The ScriptEngine must be active (initialized and not yet shut down) when destroying a ScriptRuntime!"); ScriptEngine::GetSingleton().UnRegisterRuntime(m_rt); Index: ps/trunk/source/scriptinterface/tests/test_ObjectToIDMap.h =================================================================== --- ps/trunk/source/scriptinterface/tests/test_ObjectToIDMap.h +++ ps/trunk/source/scriptinterface/tests/test_ObjectToIDMap.h @@ -1,64 +0,0 @@ -/* Copyright (C) 2016 Wildfire Games. - * This file is part of 0 A.D. - * - * 0 A.D. is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 2 of the License, or - * (at your option) any later version. - * - * 0 A.D. is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with 0 A.D. If not, see . - */ - -#include "lib/self_test.h" - -#include "scriptinterface/ScriptInterface.h" -#include "scriptinterface/ScriptRuntime.h" -#include "scriptinterface/third_party/ObjectToIDMap.h" - -class TestObjectToIDMap : public CxxTest::TestSuite -{ -public: - void test_movinggc() - { - ScriptInterface script("Test", "Test", g_ScriptRuntime); - JSContext* cx = script.GetContext(); - JSAutoRequest rq(cx); - - JS::RootedObject obj(cx, JS_NewPlainObject(cx)); - ObjectIdCache map(g_ScriptRuntime); - map.init(); - - TS_ASSERT(map.add(cx, obj, 1)); - JSObject* plainObj = obj; - // The map should contain the object we've just added - TS_ASSERT(map.has(plainObj)); - - JS_GC(g_ScriptRuntime->m_rt); - - // After a GC, the object should have been moved and plainObj should - // not be valid anymore and not be found in the map anymore. - // Obj should have an updated reference too, so it should still be found - // in the map. - // - // NOTE: It's observed behaviour that a full GC always moves an object. - // This might change in future SpiderMonkey versions. We only rely on - // that behaviour for this test. - // - // TODO: It might be a good idea to test the behaviour when only a minor - // GC runs, but there's no API for calling a minor GC yet. - TS_ASSERT(plainObj != obj); - TS_ASSERT(!map.has(plainObj)); - TS_ASSERT(map.has(obj)); - - // Finding the ID associated with the object - u32 ret(0); - TS_ASSERT(map.find(obj, ret)); - TS_ASSERT_EQUALS(ret, 1); - } -}; Index: ps/trunk/source/scriptinterface/third_party/ObjectToIDMap.h =================================================================== --- ps/trunk/source/scriptinterface/third_party/ObjectToIDMap.h +++ ps/trunk/source/scriptinterface/third_party/ObjectToIDMap.h @@ -1,132 +0,0 @@ -/* 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/. */ - - -/** - * Providing a map-like structure with JSObject pointers (actually their hash) as keys - * with correct garbage collection handling (JSObjects can move in memory). - * - * The code in this class was copied from here and modified to work in our environment. - * * https://mxr.mozilla.org/mozilla-esr38/source/js/ipc/JavaScriptShared.h - * * https://mxr.mozilla.org/mozilla-esr38/source/js/ipc/JavaScriptShared.cpp - * - * When updating SpiderMonkey, you most likely have to reintegrate an updated version - * of the class(es) in this file. The best way is probably to get a diff between the - * original files and integrate that because this file is heavily modified from the - * original version. - */ - -#ifndef INCLUDED_OBJECTTOIDMAP -#define INCLUDED_OBJECTTOIDMAP - -#include "scriptinterface/ScriptRuntime.h" -#include "scriptinterface/ScriptTypes.h" -#include - -// Map JSObjects -> ids -template -class ObjectIdCache -{ - typedef js::PointerHasher Hasher; - typedef js::HashMap Table; - - NONCOPYABLE(ObjectIdCache); - -public: - ObjectIdCache(shared_ptr rt) - : table_(nullptr), m_rt(rt) - { - } - - ~ObjectIdCache() - { - if (table_) - { - m_rt->AddDeferredFinalizationObject(std::shared_ptr((void*)table_, DeleteTable)); - table_ = nullptr; - JS_RemoveExtraGCRootsTracer(m_rt->m_rt, ObjectIdCache::Trace, this); - } - } - - bool init() - { - if (table_) - return true; - - table_ = new Table(js::SystemAllocPolicy()); - JS_AddExtraGCRootsTracer(m_rt->m_rt, ObjectIdCache::Trace, this); - return table_ && table_->init(32); - } - - void trace(JSTracer* trc) - { - for (typename Table::Enum e(*table_); !e.empty(); e.popFront()) - { - JSObject* obj = e.front().key(); - JS_CallUnbarrieredObjectTracer(trc, &obj, "ipc-object"); - if (obj != e.front().key()) - e.rekeyFront(obj); - } - } - -// TODO sweep? - - bool find(JSObject* obj, T& ret) - { - typename Table::Ptr p = table_->lookup(obj); - if (!p) - return false; - ret = p->value(); - return true; - } - - bool add(JSContext* cx, JSObject* obj, T id) - { - if (!table_->put(obj, id)) - return false; - JS_StoreObjectPostBarrierCallback(cx, keyMarkCallback, obj, table_); - return true; - } - - void remove(JSObject* obj) - { - table_->remove(obj); - } - -// TODO clear? - - bool empty() - { - return table_->empty(); - } - - bool has(JSObject* obj) - { - return table_->has(obj); - } - -private: - static void keyMarkCallback(JSTracer* trc, JSObject* key, void* data) - { - Table* table = static_cast(data); - JSObject* prior = key; - JS_CallUnbarrieredObjectTracer(trc, &key, "ObjectIdCache::table_ key"); - table->rekeyIfMoved(prior, key); - } - - static void Trace(JSTracer* trc, void* data) - { - reinterpret_cast(data)->trace(trc); - } - - static void DeleteTable(void* table) - { - delete (Table*)table; - } - - shared_ptr m_rt; - Table* table_; -}; - -#endif // INCLUDED_OBJECTTOIDMAP Index: ps/trunk/source/simulation2/components/CCmpAIManager.cpp =================================================================== --- ps/trunk/source/simulation2/components/CCmpAIManager.cpp +++ ps/trunk/source/simulation2/components/CCmpAIManager.cpp @@ -32,6 +32,7 @@ #include "ps/scripting/JSInterface_VFS.h" #include "ps/TemplateLoader.h" #include "ps/Util.h" +#include "scriptinterface/ScriptRuntime.h" #include "simulation2/components/ICmpAIInterface.h" #include "simulation2/components/ICmpCommandQueue.h" #include "simulation2/components/ICmpObstructionManager.h" Index: ps/trunk/source/simulation2/components/tests/test_CommandQueue.h =================================================================== --- ps/trunk/source/simulation2/components/tests/test_CommandQueue.h +++ ps/trunk/source/simulation2/components/tests/test_CommandQueue.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2015 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 @@ -49,7 +49,7 @@ TS_ASSERT(test.GetScriptInterface().Eval("([1,2,3])", &cmd)); cmp->PushLocalCommand(1, cmd); - TS_ASSERT(test.GetScriptInterface().Eval("({x:4})", &cmd)); + TS_ASSERT(test.GetScriptInterface().Eval("({\"x\":4})", &cmd)); cmp->PushLocalCommand(-1, cmd); test.Roundtrip(); @@ -57,7 +57,7 @@ // Process the first two commands cmp->FlushTurn(empty); - TS_ASSERT(test.GetScriptInterface().Eval("({y:5})", &cmd)); + TS_ASSERT(test.GetScriptInterface().Eval("({\"y\":5})", &cmd)); cmp->PushLocalCommand(10, cmd); // Process the next command @@ -69,7 +69,7 @@ test.Roundtrip(); std::string output; - TS_ASSERT(test.GetScriptInterface().Eval("uneval(cmds)", output)); - TS_ASSERT_STR_EQUALS(output, "[[1, [1, 2, 3]], [-1, {x:4}], [10, {y:5}]]"); + TS_ASSERT(test.GetScriptInterface().Eval("JSON.stringify(cmds)", output)); + TS_ASSERT_STR_EQUALS(output, "[[1,[1,2,3]],[-1,{\"x\":4}],[10,{\"y\":5}]]"); } }; Index: ps/trunk/source/simulation2/serialization/BinarySerializer.h =================================================================== --- ps/trunk/source/simulation2/serialization/BinarySerializer.h +++ ps/trunk/source/simulation2/serialization/BinarySerializer.h @@ -20,8 +20,6 @@ #include "ISerializer.h" -#include "scriptinterface/third_party/ObjectToIDMap.h" - #include "lib/byte_order.h" #include "lib/allocators/arena.h" @@ -91,9 +89,9 @@ const ScriptInterface& m_ScriptInterface; ISerializer& m_Serializer; - ObjectIdCache m_ScriptBackrefs; - u32 m_ScriptBackrefsNext; - u32 GetScriptBackrefTag(JS::HandleObject obj); + JS::PersistentRootedSymbol m_ScriptBackrefSymbol; + i32 m_ScriptBackrefsNext; + i32 GetScriptBackrefTag(JS::HandleObject obj); }; /** Index: ps/trunk/source/simulation2/serialization/BinarySerializer.cpp =================================================================== --- ps/trunk/source/simulation2/serialization/BinarySerializer.cpp +++ ps/trunk/source/simulation2/serialization/BinarySerializer.cpp @@ -55,10 +55,12 @@ } CBinarySerializerScriptImpl::CBinarySerializerScriptImpl(const ScriptInterface& scriptInterface, ISerializer& serializer) : - m_ScriptInterface(scriptInterface), m_Serializer(serializer), m_ScriptBackrefs(scriptInterface.GetRuntime()), - m_ScriptBackrefsNext(1) + m_ScriptInterface(scriptInterface), m_Serializer(serializer), m_ScriptBackrefsNext(0) { - m_ScriptBackrefs.init(); + JSContext* cx = m_ScriptInterface.GetContext(); + JSAutoRequest rq(cx); + + m_ScriptBackrefSymbol.init(cx, JS::NewSymbol(cx, nullptr)); } void CBinarySerializerScriptImpl::HandleScriptVal(JS::HandleValue val) @@ -89,11 +91,11 @@ JS::RootedObject obj(cx, &val.toObject()); // If we've already serialized this object, just output a reference to it - u32 tag = GetScriptBackrefTag(obj); - if (tag) + i32 tag = GetScriptBackrefTag(obj); + if (tag != -1) { m_Serializer.NumberU8_Unbounded("type", SCRIPT_TYPE_BACKREF); - m_Serializer.NumberU32_Unbounded("tag", tag); + m_Serializer.NumberI32("tag", tag, 0, JSVAL_INT_MAX); break; } @@ -406,27 +408,39 @@ } } -u32 CBinarySerializerScriptImpl::GetScriptBackrefTag(JS::HandleObject obj) +i32 CBinarySerializerScriptImpl::GetScriptBackrefTag(JS::HandleObject obj) { // To support non-tree structures (e.g. "var x = []; var y = [x, x];"), we need a way // to indicate multiple references to one object(/array). So every time we serialize a - // new object, we give it a new non-zero tag; when we serialize it a second time we just - // refer to that tag. + // new object, we give it a new tag; when we serialize it a second time we just refer + // to that tag. // - // The tags are stored in a map. Maybe it'd be more efficient to store it inline in the object - // somehow? but this works okay for now - - // If it was already there, return the tag - u32 tag; - if (m_ScriptBackrefs.find(obj, tag)) - return tag; + // Tags are stored on the object. To avoid overwriting any existing property, + // they are saved as a uniquely-named, non-enumerable property (the serializer's unique symbol). JSContext* cx = m_ScriptInterface.GetContext(); JSAutoRequest rq(cx); - m_ScriptBackrefs.add(cx, obj, m_ScriptBackrefsNext); + JS::RootedValue symbolValue(cx, JS::SymbolValue(m_ScriptBackrefSymbol)); + JS::RootedId symbolId(cx); + ENSURE(JS_ValueToId(cx, symbolValue, &symbolId)); + + JS::RootedValue tagValue(cx); + + // If it was already there, return the tag + bool tagFound; + ENSURE(JS_HasPropertyById(cx, obj, symbolId, &tagFound)); + if (tagFound) + { + ENSURE(JS_GetPropertyById(cx, obj, symbolId, &tagValue)); + ENSURE(tagValue.isInt32()); + return tagValue.toInt32(); + } + + tagValue = JS::Int32Value(m_ScriptBackrefsNext); + JS_SetPropertyById(cx, obj, symbolId, tagValue); - m_ScriptBackrefsNext++; + ++m_ScriptBackrefsNext; // Return a non-tag number so callers know they need to serialize the object - return 0; + return -1; } Index: ps/trunk/source/simulation2/serialization/StdDeserializer.h =================================================================== --- ps/trunk/source/simulation2/serialization/StdDeserializer.h +++ ps/trunk/source/simulation2/serialization/StdDeserializer.h @@ -51,9 +51,8 @@ void ReadStringUTF16(const char* name, utf16string& str); virtual void AddScriptBackref(JS::HandleObject obj); - virtual void GetScriptBackref(u32 tag, JS::MutableHandleObject ret); + virtual void GetScriptBackref(size_t tag, JS::MutableHandleObject ret); std::vector > m_ScriptBackrefs; - JS::PersistentRooted m_dummyObject; const ScriptInterface& m_ScriptInterface; Index: ps/trunk/source/simulation2/serialization/StdDeserializer.cpp =================================================================== --- ps/trunk/source/simulation2/serialization/StdDeserializer.cpp +++ ps/trunk/source/simulation2/serialization/StdDeserializer.cpp @@ -28,18 +28,9 @@ #include "lib/byte_order.h" CStdDeserializer::CStdDeserializer(const ScriptInterface& scriptInterface, std::istream& stream) : - m_ScriptInterface(scriptInterface), m_Stream(stream), - m_dummyObject(scriptInterface.GetJSRuntime()) + m_ScriptInterface(scriptInterface), m_Stream(stream) { - JSContext* cx = m_ScriptInterface.GetContext(); - JSAutoRequest rq(cx); - JS_AddExtraGCRootsTracer(m_ScriptInterface.GetJSRuntime(), CStdDeserializer::Trace, this); - - // Add a dummy tag because the serializer uses the tag 0 to indicate that a value - // needs to be serialized and then tagged - m_dummyObject = JS_NewPlainObject(cx); - m_ScriptBackrefs.push_back(JS::Heap(m_dummyObject)); } CStdDeserializer::~CStdDeserializer() @@ -111,7 +102,7 @@ m_ScriptBackrefs.push_back(JS::Heap(obj)); } -void CStdDeserializer::GetScriptBackref(u32 tag, JS::MutableHandleObject ret) +void CStdDeserializer::GetScriptBackref(size_t tag, JS::MutableHandleObject ret) { ENSURE(m_ScriptBackrefs.size() > tag); ret.set(m_ScriptBackrefs[tag]); @@ -217,8 +208,8 @@ } case SCRIPT_TYPE_BACKREF: { - u32 tag; - NumberU32_Unbounded("tag", tag); + i32 tag; + NumberI32("tag", tag, 0, JSVAL_INT_MAX); JS::RootedObject obj(cx); GetScriptBackref(tag, &obj); if (!obj) Index: ps/trunk/source/simulation2/tests/test_Serializer.h =================================================================== --- ps/trunk/source/simulation2/tests/test_Serializer.h +++ ps/trunk/source/simulation2/tests/test_Serializer.h @@ -689,7 +689,7 @@ "\x05" // SCRIPT_TYPE_INT "\x02\0\0\0" // 2 "\x08" // SCRIPT_TYPE_BACKREF - "\x02\0\0\0" // ref. to object #2, i.e. "b", with #1 being "a" + "\x01\0\0\0" // ref. to object #1, i.e. "b", with #0 being "a" ); } @@ -711,7 +711,7 @@ "\x01\0\0\0" // num props "\x01\x03\0\0\0" "bar" // "bar" "\x08" // SCRIPT_TYPE_BACKREF - "\x02\0\0\0" // ref to object #2, i.e. "b", with #1 being "a" + "\x01\0\0\0" // ref to object #1, i.e. "b", with #0 being "a" ); }