[gjs/ewlsh/top-level-await-mainloop] Add implicit main loop for dynamic imports




commit 1692279580c6898ab7451031c75a86d282ad1f89
Author: Evan Welsh <contact evanwelsh com>
Date:   Sat Nov 6 18:34:31 2021 -0700

    Add implicit main loop for dynamic imports

 gjs/context-private.h                             |  4 +++
 gjs/context.cpp                                   | 13 ++++++-
 gjs/internal.cpp                                  |  9 ++++-
 gjs/mainloop.cpp                                  | 41 +++++++++++++++++++++
 gjs/mainloop.h                                    | 44 +++++++++++++++++++++++
 gjs/module.cpp                                    |  9 +++++
 installed-tests/scripts/testCommandLineModules.sh | 19 +++++++++-
 meson.build                                       |  1 +
 tools/run_iwyu.sh                                 |  3 +-
 9 files changed, 139 insertions(+), 4 deletions(-)
---
diff --git a/gjs/context-private.h b/gjs/context-private.h
index 0499d286..48394b4d 100644
--- a/gjs/context-private.h
+++ b/gjs/context-private.h
@@ -37,6 +37,7 @@
 #include "gjs/context.h"
 #include "gjs/jsapi-util.h"
 #include "gjs/macros.h"
+#include "gjs/mainloop.h"
 #include "gjs/profiler.h"
 #include "gjs/promise.h"
 
@@ -81,6 +82,7 @@ class GjsContextPrivate : public JS::JobQueue {
 
     JobQueueStorage m_job_queue;
     Gjs::PromiseJobDispatcher m_dispatcher;
+    Gjs::MainLoop m_main_loop;
 
     std::vector<std::pair<DestroyNotify, void*>> m_destroy_notifications;
     std::vector<Gjs::Closure::Ptr> m_async_closures;
@@ -176,6 +178,8 @@ class GjsContextPrivate : public JS::JobQueue {
     [[nodiscard]] JSObject* internal_global() const {
         return m_internal_global.get();
     }
+    void main_loop_hold() { m_main_loop.hold(); }
+    void main_loop_release() { m_main_loop.release(); }
     [[nodiscard]] GjsProfiler* profiler() const { return m_profiler; }
     [[nodiscard]] const GjsAtoms& atoms() const { return *m_atoms; }
     [[nodiscard]] bool destroying() const { return m_destroying.load(); }
diff --git a/gjs/context.cpp b/gjs/context.cpp
index 9a75d596..e8961056 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -75,6 +75,7 @@
 #include "gjs/importer.h"
 #include "gjs/internal.h"
 #include "gjs/jsapi-util.h"
+#include "gjs/mainloop.h"
 #include "gjs/mem.h"
 #include "gjs/module.h"
 #include "gjs/native.h"
@@ -1252,6 +1253,9 @@ bool GjsContextPrivate::eval(const char* script, ssize_t script_len,
     JS::RootedValue retval(m_cx);
     bool ok = eval_with_scope(nullptr, script, script_len, filename, &retval);
 
+    if (ok)
+        m_main_loop.spin(this);
+
     /* The promise job queue should be drained even on error, to finish
      * outstanding async tasks before the context is torn down. Drain after
      * uncaught exceptions have been reported since draining runs callbacks. */
@@ -1268,13 +1272,17 @@ bool GjsContextPrivate::eval(const char* script, ssize_t script_len,
     }
 
     if (exit_status_p) {
+        uint8_t out_code;
         if (retval.isInt32()) {
             int code = retval.toInt32();
             gjs_debug(GJS_DEBUG_CONTEXT,
                       "Script returned integer code %d", code);
             *exit_status_p = code;
+        } else if (should_exit(&out_code)) {
+            *exit_status_p = out_code;
         } else {
-            /* Assume success if no integer was returned */
+            // Assume success if no integer was returned and should exit isn't
+            // set
             *exit_status_p = 0;
         }
     }
@@ -1310,6 +1318,9 @@ bool GjsContextPrivate::eval_module(const char* identifier,
 
     bool ok = JS::ModuleEvaluate(m_cx, obj);
 
+    if (ok)
+        m_main_loop.spin(this);
+
     /* The promise job queue should be drained even on error, to finish
      * outstanding async tasks before the context is torn down. Drain after
      * uncaught exceptions have been reported since draining runs callbacks.
diff --git a/gjs/internal.cpp b/gjs/internal.cpp
index ed856ac4..a8ed70fc 100644
--- a/gjs/internal.cpp
+++ b/gjs/internal.cpp
@@ -39,6 +39,7 @@
 #include "gjs/importer.h"
 #include "gjs/jsapi-util-args.h"
 #include "gjs/jsapi-util.h"
+#include "gjs/mainloop.h"
 #include "gjs/mem-private.h"
 #include "gjs/module.h"
 #include "gjs/native.h"
@@ -503,7 +504,10 @@ class PromiseData {
 static void load_async_callback(GObject* file, GAsyncResult* res, void* data) {
     std::unique_ptr<PromiseData> promise(PromiseData::from_ptr(data));
 
-    JSAutoRealm ac(promise->cx, gjs_get_import_global(promise->cx));
+    GjsContextPrivate* gjs = GjsContextPrivate::from_cx(promise->cx);
+    gjs->main_loop_release();
+
+    JSAutoRealm ar(promise->cx, gjs->global());
 
     char* contents;
     size_t length;
@@ -551,6 +555,9 @@ static bool load_async_executor(JSContext* cx, unsigned argc, JS::Value* vp) {
 
     auto* data = new PromiseData(cx, JS_GetObjectFunction(resolve),
                                  JS_GetObjectFunction(reject));
+
+    // Hold the main loop until this function resolves...
+    GjsContextPrivate::from_cx(cx)->main_loop_hold();
     g_file_load_contents_async(file, nullptr, load_async_callback, data);
 
     args.rval().setUndefined();
diff --git a/gjs/mainloop.cpp b/gjs/mainloop.cpp
new file mode 100644
index 00000000..19e8f722
--- /dev/null
+++ b/gjs/mainloop.cpp
@@ -0,0 +1,41 @@
+/* -*- mode: C++; c-basic-offset: 4; indent-tabs-mode: nil; -*- */
+// SPDX-License-Identifier: MIT OR LGPL-2.0-or-later
+// SPDX-FileCopyrightText: 2021 Evan Welsh <contact evanwelsh com>
+
+#include <config.h>
+
+#include <glib.h>
+
+#include "gjs/context-private.h"
+#include "gjs/jsapi-util.h"
+#include "gjs/mainloop.h"
+
+namespace Gjs {
+
+void MainLoop::spin(GjsContextPrivate* gjs) {
+    // Check if System.exit() has been called.
+    if (gjs->should_exit(nullptr))
+        return;
+
+    GjsAutoPointer<GMainContext, GMainContext, g_main_context_unref>
+        main_context(g_main_context_ref_thread_default());
+
+    do {
+        if (gjs->should_exit(nullptr))
+            break;
+
+        bool blocking = can_block();
+
+        // Only run the loop if there are pending jobs.
+        if (g_main_context_pending(main_context)) {
+            g_main_context_iteration(main_context, blocking);
+        }
+    } while (
+        // If System.exit() has not been called
+        !gjs->should_exit(nullptr) &&
+        // and there are pending sources or the job queue is not empty
+        // continue spinning the event loop.
+        (can_block() || !gjs->empty()));
+}
+
+};  // namespace Gjs
diff --git a/gjs/mainloop.h b/gjs/mainloop.h
new file mode 100644
index 00000000..3dcd58d6
--- /dev/null
+++ b/gjs/mainloop.h
@@ -0,0 +1,44 @@
+/* -*- mode: C++; c-basic-offset: 4; indent-tabs-mode: nil; -*- */
+// SPDX-License-Identifier: MIT OR LGPL-2.0-or-later
+// SPDX-FileCopyrightText: 2021 Evan Welsh <contact evanwelsh com>
+
+#pragma once
+
+#include <config.h>
+
+#include <glib.h>
+
+class GjsContextPrivate;
+
+namespace Gjs {
+
+class MainLoop {
+    // grefcounts start at one and become invalidated when they are decremented
+    // to zero. So the actual hold count is equal to the "ref" count minus 1.
+    // We nonetheless use grefcount here because it takes care of dealing with
+    // integer overflow for us.
+    grefcount m_hold_count;
+
+ public:
+    MainLoop() { g_ref_count_init(&m_hold_count); }
+    ~MainLoop() { g_assert(g_ref_count_compare(&m_hold_count, 1)); }
+
+    bool can_block() {
+        bool zero [[maybe_unused]] = g_ref_count_compare(&m_hold_count, 0);
+        g_assert(!zero && "main loop released too many times");
+
+        // If the reference count is not zero or one, the loop is being held.
+        return !g_ref_count_compare(&m_hold_count, 1);
+    }
+
+    void hold() { g_ref_count_inc(&m_hold_count); }
+
+    void release() {
+        bool zero [[maybe_unused]] = g_ref_count_dec(&m_hold_count);
+        g_assert(!zero && "main loop released too many times");
+    }
+
+    void spin(GjsContextPrivate*);
+};
+
+};  // namespace Gjs
diff --git a/gjs/module.cpp b/gjs/module.cpp
index c92bd05f..9e7ab6a0 100644
--- a/gjs/module.cpp
+++ b/gjs/module.cpp
@@ -535,6 +535,9 @@ JSObject* gjs_module_resolve(JSContext* cx, JS::HandleValue importingModulePriv,
 // fails in fetching the stashed values, since that would be a serious GJS bug.
 GJS_JSAPI_RETURN_CONVENTION
 static bool finish_import(JSContext* cx, const JS::CallArgs& args) {
+    GjsContextPrivate* priv = GjsContextPrivate::from_cx(cx);
+    priv->main_loop_release();
+
     JS::Value callback_priv = js::GetFunctionNativeReserved(&args.callee(), 0);
     g_assert(callback_priv.isObject() && "Wrong private value");
     JS::RootedObject callback_data(cx, &callback_priv.toObject());
@@ -650,6 +653,12 @@ bool gjs_dynamic_module_resolve(JSContext* cx,
         return JS::FinishDynamicModuleImport(cx, importing_module_priv,
                                              specifier, internal_promise);
 
+    // TODO: The narrower hold within our file resolver seems to work
+    // but we should validate that the code always executes synchronously
+    // prior to the file resolver...
+    GjsContextPrivate* priv = GjsContextPrivate::from_cx(cx);
+    priv->main_loop_hold();
+
     JS::RootedObject resolved(
         cx, JS_GetFunctionObject(js::NewFunctionWithReserved(
                 cx, import_resolved, 1, 0, "async import resolved")));
diff --git a/installed-tests/scripts/testCommandLineModules.sh 
b/installed-tests/scripts/testCommandLineModules.sh
index c075b216..fe2e9eb6 100755
--- a/installed-tests/scripts/testCommandLineModules.sh
+++ b/installed-tests/scripts/testCommandLineModules.sh
@@ -61,6 +61,23 @@ EOF
 $gjs doubledynamic.js
 report "ensure dynamic imports load even if the same import resolves elsewhere first"
 
-rm -f doubledynamic.js doubledynamicImportee.js
+cat <<EOF >dynamicImplicitMainloopImportee.js
+export const EXIT_CODE = 21;
+EOF
+
+cat <<EOF >dynamicImplicitMainloop.js
+import("./dynamicImplicitMainloopImportee.js")
+    .then(({ EXIT_CODE }) => {
+      imports.system.exit(EXIT_CODE);
+    });
+EOF
+
+$gjs dynamicImplicitMainloop.js
+test $? -eq 21
+report "ensure dynamic imports resolve without an explicit mainloop"
+
+
+# rm -f doubledynamic.js doubledynamicImportee.js \
+#       dynamicImplicitMainloop.js dynamicImplicitMainloopImportee.js
 
 echo "1..$total"
diff --git a/meson.build b/meson.build
index 437b3fd3..53715bd1 100644
--- a/meson.build
+++ b/meson.build
@@ -418,6 +418,7 @@ libgjs_sources = [
     'gjs/global.cpp', 'gjs/global.h',
     'gjs/importer.cpp', 'gjs/importer.h',
     'gjs/internal.cpp', 'gjs/internal.h',
+    'gjs/mainloop.cpp', 'gjs/mainloop.h',
     'gjs/mem.cpp', 'gjs/mem-private.h',
     'gjs/module.cpp', 'gjs/module.h',
     'gjs/native.cpp', 'gjs/native.h',
diff --git a/tools/run_iwyu.sh b/tools/run_iwyu.sh
index 7e1db246..520b8549 100755
--- a/tools/run_iwyu.sh
+++ b/tools/run_iwyu.sh
@@ -70,7 +70,8 @@ for FILE in $SRCDIR/gi/*.cpp $SRCDIR/gjs/atoms.cpp $SRCDIR/gjs/byteArray.cpp \
     $SRCDIR/gjs/coverage.cpp $SRCDIR/gjs/debugger.cpp \
     $SRCDIR/gjs/deprecation.cpp $SRCDIR/gjs/error-types.cpp \
     $SRCDIR/gjs/engine.cpp $SRCDIR/gjs/global.cpp $SRCDIR/gjs/importer.cpp \
-    $SRCDIR/gjs/jsapi-util*.cpp $SRCDIR/gjs/module.cpp $SRCDIR/gjs/native.cpp \
+    $SRCDIR/gjs/jsapi-util*.cpp $SRCDIR/gjs/mainloop.cpp \
+    $SRCDIR/gjs/module.cpp $SRCDIR/gjs/native.cpp \
     $SRCDIR/gjs/objectbox.cpp $SRCDIR/gjs/promise.cpp $SRCDIR/gjs/stack.cpp \
     $SRCDIR/modules/cairo-*.cpp $SRCDIR/modules/console.cpp \
     $SRCDIR/modules/print.cpp $SRCDIR/modules/system.cpp $SRCDIR/test/*.cpp \


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