Ticket #3708: JS_ShutDown_SVN_1.0.diff

source/main.cpp

@@ -73 +73 @@
 #include "graphics/TextureManager.h"
 #include "gui/GUIManager.h"
 #include "renderer/Renderer.h"
+#include "scriptinterface/ScriptEngine.h"
 #include "simulation2/Simulation2.h"
 
 #if OS_UNIX
@@ -422 +423 @@
         return;
     }
 
-    // We need to initialise libxml2 in the main thread before
-    // any thread uses it. So initialise it here before we
-    // might run Atlas.
+    // We need to initialize libxml2 and SpiderMonkey in the main thread before
+    // any thread uses them. So initialize them here before we might run Atlas.
     CXeromyces::Startup();
+    ScriptEngine scriptEngine;
 
     // Atlas handles the whole init/shutdown/etc sequence by itself;
     if (ATLAS_RunIfOnCmdLine(args, false))

source/scriptinterface/ScriptEngine.h

@@ +1 @@
+/* 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 <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef INCLUDED_SCRIPTENGINE
+#define INCLUDED_SCRIPTENGINE
+
+#include "ScriptTypes.h"
+#include "ps/Singleton.h"
+
+/**
+ * A class using the RAII (Resource Acquisition Is Initialization) idiom to manage initialization
+ * and shutdown of the SpiderMonkey script engine. It also keeps a count of active script runtimes
+ * in order to validate the following constraints:
+ *  1. JS_Init must be called before any ScriptRuntimes are initialized
+ *  2. JS_Shutdown must be called after all ScriptRuntimes have been destroyed
+ */
+
+class ScriptEngine : public Singleton<ScriptEngine>
+{
+public:
+    ScriptEngine()
+    {
+        ENSURE(m_Runtimes.size() == 0 && "JS_Init must be called before any runtimes are created!");
+        JS_Init();
+    }
+
+    ~ScriptEngine()
+    {
+        ENSURE(m_Runtimes.size() == 0 && "All runtimes must be destroyed before calling JS_ShutDown!");
+        JS_ShutDown();
+    }
+
+    void RegisterRuntime(const JSRuntime* rt) { m_Runtimes.push_back(rt); }
+    void UnRegisterRuntime(const JSRuntime* rt) { m_Runtimes.remove(rt); }
+
+private:
+
+    std::list<const JSRuntime*> m_Runtimes;
+};
+
+#endif // INCLUDED_SCRIPTENGINE

source/scriptinterface/ScriptInterface.cpp

@@ -433 +433 @@
     }
 }
 
-void ScriptInterface::ShutDown()
-{
-    JS_ShutDown();
-}
-
 void ScriptInterface::SetCallbackData(void* pCBData)
 {
     m_CxPrivate.pCBData = pCBData;

source/scriptinterface/ScriptInterface.h

@@ -97 +97 @@
 
     ~ScriptInterface();
 
-    /**
-     * Shut down the JS system to clean up memory. Must only be called when there
-     * are no ScriptInterfaces alive.
-     */
-    static void ShutDown();
-
     struct CxPrivate
     {
         ScriptInterface* pScriptInterface; // the ScriptInterface object the current context belongs to

source/scriptinterface/ScriptRuntime.cpp

@@ -21 +21 @@
 
 #include "ps/GameSetup/Config.h"
 #include "ps/Profile.h"
+#include "scriptinterface/ScriptEngine.h"
 
 
 void GCSliceCallbackHook(JSRuntime* UNUSED(rt), JS::GCProgress progress, const JS::GCDescription& UNUSED(desc))
@@ -104 +105 @@
     m_FinalizationListObjectIdCache.push_back(obj);
 }
 
-bool ScriptRuntime::m_Initialized = false;
-
 ScriptRuntime::ScriptRuntime(shared_ptr<ScriptRuntime> parentRuntime, int runtimeSize, int heapGrowthBytesGCTrigger):
     m_LastGCBytes(0),
     m_LastGCCheck(0.0f),
@@ -112 +111 @@
     m_HeapGrowthBytesGCTrigger(heapGrowthBytesGCTrigger),
     m_RuntimeSize(runtimeSize)
 {
-    if (!m_Initialized)
-    {
-        ENSURE(JS_Init());
-        m_Initialized = true;
-    }
+    ENSURE(ScriptEngine::IsInitialised() && "The ScriptEngine must be initialized before constructing any ScriptRuntimes!");
 
     JSRuntime* parentJSRuntime = parentRuntime ? parentRuntime->m_rt : nullptr;
     m_rt = JS_NewRuntime(runtimeSize, JS_USE_HELPER_THREADS, parentJSRuntime);
@@ -135 +130 @@
     
     m_dummyContext = JS_NewContext(m_rt, STACK_CHUNK_SIZE);
     ENSURE(m_dummyContext);
+
+    ScriptEngine::GetSingleton().RegisterRuntime(m_rt);
 }
 
 ScriptRuntime::~ScriptRuntime()
@@ -143 +140 @@
     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 (initiaized and not yet shut down) when destroying a ScriptRuntime!");
+    ScriptEngine::GetSingleton().UnRegisterRuntime(m_rt);
 }
 
 void ScriptRuntime::RegisterContext(JSContext* cx)

source/scriptinterface/ScriptRuntime.h

@@ -79 +79 @@
     
     std::list<JSContext*> m_Contexts;
     std::vector<std::shared_ptr<void> > m_FinalizationListObjectIdCache;
-    static bool m_Initialized;
 
     int m_RuntimeSize;
     int m_HeapGrowthBytesGCTrigger;

source/test_setup.cpp

@@ -1 @@
-/* Copyright (C) 2014 Wildfire Games.
+/* 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
@@ -36 +36 @@
 #include "lib/timer.h"
 #include "lib/sysdep/sysdep.h"
 #include "ps/Profiler2.h"
+#include "scriptinterface/ScriptEngine.h"
 #include "scriptinterface/ScriptInterface.h"
 
 class LeakReporter : public CxxTest::GlobalFixture
@@ -42 +43 @@
 {
     virtual bool tearDownWorld()
     {
-        // Shut down JS to prevent leak reports from it
-        ScriptInterface::ShutDown();
-
         // Enable leak reporting on exit.
         // (This is done in tearDownWorld so that it doesn't report 'leaks'
         // if the program is aborted before finishing cleanly.)
@@ -81 +79 @@
         ThreadUtil::SetMainThread();
 
         g_Profiler2.Initialise();
+        m_ScriptEngine = new ScriptEngine;
         g_ScriptRuntime = ScriptInterface::CreateRuntime();
 
         return true;
@@ -89 +88 @@
     virtual bool tearDownWorld()
     {
         g_ScriptRuntime.reset();
+        delete m_ScriptEngine;
         g_Profiler2.Shutdown();
 
         return true;
     }
+
+private:
+
+    // We're doing the initialization and shutdown of the ScriptEngine explicitly here
+    // to make sure it's only initialized when setUpWorld is called.
+    ScriptEngine* m_ScriptEngine;
 };
 
 static LeakReporter leakReporter;