[gjs/ewlsh/nova-repl] Refactor GjsInit to avoid re-entrancy issues




commit c910cba53f632d467fdb4b2e853f1756e9a728eb
Author: Evan Welsh <contact evanwelsh com>
Date:   Fri Sep 3 14:08:18 2021 -0700

    Refactor GjsInit to avoid re-entrancy issues

 gjs/console.cpp                                    |  3 +-
 gjs/context.cpp                                    |  4 +-
 gjs/engine.cpp                                     | 47 +++++++++++-----------
 gjs/engine.h                                       | 19 +++++++++
 gjs/importer.cpp                                   |  4 +-
 installed-tests/js/jsunit.gresources.xml           |  2 +-
 .../badOverrides/{Gio.js => GjsTestTools.js}       |  0
 installed-tests/js/testImporter.js                 |  2 +-
 modules/esm/repl.js                                |  6 +--
 9 files changed, 53 insertions(+), 34 deletions(-)
---
diff --git a/gjs/console.cpp b/gjs/console.cpp
index d51a98ac..fdb3cc7e 100644
--- a/gjs/console.cpp
+++ b/gjs/console.cpp
@@ -302,12 +302,11 @@ main(int argc, char **argv)
             exec_as_module = true;
             filename =
                 "resource:///org/gnome/gjs/modules/esm/_bootstrap/repl.js";
-            interactive_mode = true;
         } else {
             script = g_strdup(
                 "const Console = imports.console; Console.interact();");
-            filename = "<stdin>";
             len = strlen(script);
+            filename = "<stdin>";
         }
 
         program_name = gjs_argv[0];
diff --git a/gjs/context.cpp b/gjs/context.cpp
index d8992a58..db76cc0c 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -312,8 +312,8 @@ gjs_context_class_init(GjsContextClass *klass)
     /* For GjsPrivate */
     if (!g_getenv("GJS_USE_UNINSTALLED_FILES")) {
 #ifdef G_OS_WIN32
-        extern HMODULE gjs_dll;
-        char *basedir = g_win32_get_package_installation_directory_of_module (gjs_dll);
+        char* basedir = g_win32_get_package_installation_directory_of_module(
+            GjsInit::dll());
         char *priv_typelib_dir = g_build_filename (basedir, "lib", "gjs", "girepository-1.0", NULL);
         g_free (basedir);
 #else
diff --git a/gjs/engine.cpp b/gjs/engine.cpp
index 0977bca8..82dda40f 100644
--- a/gjs/engine.cpp
+++ b/gjs/engine.cpp
@@ -76,9 +76,22 @@ class GjsSourceHook : public js::SourceHook {
     }
 };
 
+GjsInit::GjsInit() {
+    if (!JS_Init())
+        g_error("Could not initialize Javascript");
+}
+
+GjsInit::~GjsInit() { JS_ShutDown(); }
+
 #ifdef G_OS_WIN32
-HMODULE gjs_dll;
-static bool gjs_is_inited = false;
+GjsInit& GjsInit::initialize(HINSTANCE dll) {
+    static GjsInit init;
+    init.dll = dll;
+
+    return init;
+}
+
+static HMODULE GjsInit::dll() { return GjsInit::initialize()->m_dll; }
 
 BOOL WINAPI
 DllMain (HINSTANCE hinstDLL,
@@ -88,13 +101,12 @@ LPVOID    lpvReserved)
   switch (fdwReason)
   {
   case DLL_PROCESS_ATTACH:
-    gjs_dll = hinstDLL;
-    gjs_is_inited = JS_Init();
-    break;
+      GjsInit::initialize(hinstDLL);
+      break;
 
   case DLL_THREAD_DETACH:
-    JS_ShutDown ();
-    break;
+      GjsInit::initialize().~GjsInit();
+      break;
 
   default:
     /* do nothing */
@@ -103,27 +115,16 @@ LPVOID    lpvReserved)
 
   return TRUE;
 }
-
 #else
-class GjsInit {
-public:
-    GjsInit() {
-        if (!JS_Init())
-            g_error("Could not initialize Javascript");
-    }
-
-    ~GjsInit() {
-        JS_ShutDown();
-    }
+GjsInit& GjsInit::initialize() {
+    static GjsInit init;
 
-    explicit operator bool() const { return true; }
-};
-
-static GjsInit gjs_is_inited;
+    return init;
+}
 #endif
 
 JSContext* gjs_create_js_context(GjsContextPrivate* uninitialized_gjs) {
-    g_assert(gjs_is_inited);
+    g_assert(GjsInit::initialize());
     JSContext *cx = JS_NewContext(32 * 1024 * 1024 /* max bytes */);
     if (!cx)
         return nullptr;
diff --git a/gjs/engine.h b/gjs/engine.h
index e0140d6f..84811b7e 100644
--- a/gjs/engine.h
+++ b/gjs/engine.h
@@ -10,6 +10,25 @@
 class GjsContextPrivate;
 struct JSContext;
 
+class GjsInit {
+#ifdef G_OS_WIN32
+    HMODULE m_dll;
+#endif
+
+ public:
+    GjsInit();
+    ~GjsInit();
+
+#ifdef G_OS_WIN32
+    static GjsInit& initialize(HINSTANCE dll);
+    static HMODULE dll();
+#else
+    static GjsInit& initialize();
+#endif
+
+    explicit operator bool() const { return true; }
+};
+
 JSContext* gjs_create_js_context(GjsContextPrivate* uninitialized_gjs);
 
 bool gjs_load_internal_source(JSContext* cx, const char* filename, char** src,
diff --git a/gjs/importer.cpp b/gjs/importer.cpp
index ad8742b8..87b04980 100644
--- a/gjs/importer.cpp
+++ b/gjs/importer.cpp
@@ -801,8 +801,8 @@ JSFunctionSpec gjs_importer_proto_funcs[] = {
 
         /* ${datadir}/share/gjs-1.0 */
 #ifdef G_OS_WIN32
-        extern HMODULE gjs_dll;
-        char *basedir = g_win32_get_package_installation_directory_of_module (gjs_dll);
+        char* basedir = g_win32_get_package_installation_directory_of_module(
+            GjsInit::dll());
         GjsAutoChar gjs_data_dir =
             g_build_filename(basedir, "share", "gjs-1.0", nullptr);
         gjs_search_path.push_back(gjs_data_dir.get());
diff --git a/installed-tests/js/jsunit.gresources.xml b/installed-tests/js/jsunit.gresources.xml
index fea06b50..dc0a47b0 100644
--- a/installed-tests/js/jsunit.gresources.xml
+++ b/installed-tests/js/jsunit.gresources.xml
@@ -9,7 +9,7 @@
     <file>minijasmine.js</file>
     <file>modules/alwaysThrows.js</file>
     <file>modules/badOverrides/GIMarshallingTests.js</file>
-    <file>modules/badOverrides/Gio.js</file>
+    <file>modules/badOverrides/GjsTestTools.js</file>
     <file>modules/badOverrides/Regress.js</file>
     <file>modules/badOverrides/WarnLib.js</file>
     <file>modules/data.txt</file>
diff --git a/installed-tests/js/modules/badOverrides/Gio.js 
b/installed-tests/js/modules/badOverrides/GjsTestTools.js
similarity index 100%
rename from installed-tests/js/modules/badOverrides/Gio.js
rename to installed-tests/js/modules/badOverrides/GjsTestTools.js
diff --git a/installed-tests/js/testImporter.js b/installed-tests/js/testImporter.js
index 5202abce..7d9d11d6 100644
--- a/installed-tests/js/testImporter.js
+++ b/installed-tests/js/testImporter.js
@@ -33,7 +33,7 @@ describe('GI importer', function () {
         });
 
         it("throws an exception when the overrides _init isn't a function", function () {
-            expect(() => imports.gi.Gio).toThrowError(/_init/);
+            expect(() => imports.gi.GjsTestTools).toThrowError(/_init/);
         });
     });
 });
diff --git a/modules/esm/repl.js b/modules/esm/repl.js
index 698ec148..e8a08cee 100644
--- a/modules/esm/repl.js
+++ b/modules/esm/repl.js
@@ -1,8 +1,6 @@
 // SPDX-License-Identifier: MIT OR LGPL-2.0-or-later
 // SPDX-FileCopyrightText: 2021 Evan Welsh <contact evanwelsh com>
 
-import System from 'system';
-
 import GLib from 'gi://GLib';
 import Gio from 'gi://Gio';
 
@@ -657,6 +655,8 @@ export class Repl {
             this.isRaw = Console.enableRawMode();
 
             if (!this.isRaw) {
+                Console.disableRawMode();
+
                 this.input = new FallbackReplInput();
 
                 this._registerInputHandler();
@@ -716,7 +716,7 @@ export class Repl {
     replaceMainLoop(start, quit = () => {
         // Force an exit if a user doesn't define their
         // replacement mainloop's quit function.
-        System.exit(1);
+        imports.system.exit(1);
     }) {
         if (!(this.input instanceof ReplInput))
             return;


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]