Merge branch 'ewlsh/main-loop-hooks' into 'master'

Introduce runAsync() to run mainloops without blocking module resolution

Closes #468

See merge request GNOME/gjs!732

13 files changed, 266 insertions, 72 deletions

diff --git a/gjs/context-private.h b/gjs/context-private.h
index a109f579..d8b7b85d 100644
--- a/gjs/context-private.h
+++ b/gjs/context-private.h
@@ -64,6 +64,7 @@ class GjsContextPrivate : public JS::JobQueue {
  private:
     GjsContext* m_public_context;
     JSContext* m_cx;
+    JS::Heap<JSObject*> m_main_loop_hook;
     JS::Heap<JSObject*> m_global;
     JS::Heap<JSObject*> m_internal_global;
     std::thread::id m_owner_thread;
@@ -139,6 +140,7 @@ class GjsContextPrivate : public JS::JobQueue {
     void start_draining_job_queue(void);
     void stop_draining_job_queue(void);
 
+    GJS_JSAPI_RETURN_CONVENTION bool run_main_loop_hook();
     [[nodiscard]] bool handle_exit_code(bool no_sync_error_pending,
                                         const char* source_type,
                                         const char* identifier,
@@ -175,6 +177,8 @@ class GjsContextPrivate : public JS::JobQueue {
     [[nodiscard]] GjsContext* public_context() const {
         return m_public_context;
     }
+    [[nodiscard]] bool set_main_loop_hook(JSObject* callable);
+    [[nodiscard]] bool has_main_loop_hook() { return !!m_main_loop_hook; }
     [[nodiscard]] JSContext* context() const { return m_cx; }
     [[nodiscard]] JSObject* global() const { return m_global.get(); }
     [[nodiscard]] JSObject* internal_global() const {
diff --git a/gjs/context.cpp b/gjs/context.cpp
index b0442861..94a45c8b 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -352,6 +352,7 @@ void GjsContextPrivate::trace(JSTracer* trc, void* data) {
     JS::TraceEdge<JSObject*>(trc, &gjs->m_global, "GJS global object");
     JS::TraceEdge<JSObject*>(trc, &gjs->m_internal_global,
                              "GJS internal global object");
+    JS::TraceEdge<JSObject*>(trc, &gjs->m_main_loop_hook, "GJS main loop hook");
     gjs->m_atoms->trace(trc);
     gjs->m_job_queue.trace(trc);
     gjs->m_object_init_list.trace(trc);
@@ -463,6 +464,7 @@ void GjsContextPrivate::dispose(void) {
         JS_RemoveExtraGCRootsTracer(m_cx, &GjsContextPrivate::trace, this);
         m_global = nullptr;
         m_internal_global = nullptr;
+        m_main_loop_hook = nullptr;
 
         gjs_debug(GJS_DEBUG_CONTEXT, "Freeing allocated resources");
         delete m_fundamental_table;
@@ -1360,6 +1362,29 @@ bool GjsContextPrivate::handle_exit_code(bool no_sync_error_pending,
     return false;
 }
 
+bool GjsContextPrivate::set_main_loop_hook(JSObject* callable) {
+    g_assert(JS::IsCallable(callable) && "main loop hook must be a callable object");
+
+    if (!callable) {
+        m_main_loop_hook = nullptr;
+        return true;
+    }
+
+    if (m_main_loop_hook)
+        return false;
+
+    m_main_loop_hook = callable;
+    return true;
+}
+
+bool GjsContextPrivate::run_main_loop_hook() {
+    JS::RootedObject hook(m_cx, m_main_loop_hook.get());
+    m_main_loop_hook = nullptr;
+    JS::RootedValue ignored_rval(m_cx);
+    return JS::Call(m_cx, JS::NullHandleValue, hook,
+                    JS::HandleValueArray::empty(), &ignored_rval);
+}
+
 bool GjsContextPrivate::eval(const char* script, size_t script_len,
                              const char* filename, int* exit_status_p,
                              GError** error) {
@@ -1372,15 +1397,38 @@ bool GjsContextPrivate::eval(const char* script, size_t script_len,
     JS::RootedValue retval(m_cx);
     bool ok = eval_with_scope(nullptr, script, script_len, filename, &retval);
 
-    /* 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.
-     *
-     * If the main loop returns false we cannot guarantee the state
-     * of our promise queue (a module promise could be pending) so
-     * instead of draining the queue we instead just exit.
-     */
-    if (!ok || m_main_loop.spin(this)) {
+    // If there are no errors and the mainloop hook is set, call it.
+    if (ok && m_main_loop_hook)
+        ok = run_main_loop_hook();
+
+    bool exiting = false;
+
+    // Spin the internal loop until the main loop hook is set or no holds
+    // remain.
+    // If the main loop returns false we cannot guarantee the state of our
+    // promise queue (a module promise could be pending) so instead of draining
+    // draining the queue we instead just exit.
+    if (ok && !m_main_loop.spin(this)) {
+        exiting = true;
+    }
+
+    // If the hook has been set again, enter a loop until an error is
+    // encountered or the main loop is quit.
+    while (ok && !exiting && m_main_loop_hook) {
+        ok = run_main_loop_hook();
+
+        // Additional jobs could have been enqueued from the
+        // main loop hook
+        if (ok && !m_main_loop.spin(this)) {
+            exiting = true;
+        }
+    }
+
+    // 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.
+    // We do not drain if we are exiting.
+    if (!ok && !exiting) {
         JS::AutoSaveExceptionState saved_exc(m_cx);
         ok = run_jobs_fallible() && ok;
     }
@@ -1445,15 +1493,32 @@ bool GjsContextPrivate::eval_module(const char* identifier,
             on_context_module_rejected_log_exception, "context");
     }
 
-    /* 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.
-     *
-     * If the main loop returns false we cannot guarantee the state
-     * of our promise queue (a module promise could be pending) so
-     * instead of draining the queue we instead just exit.
-     */
-    if (!ok || m_main_loop.spin(this)) {
+    bool exiting = false;
+
+    do {
+        // If there are no errors and the mainloop hook is set, call it.
+        if (ok && m_main_loop_hook)
+            ok = run_main_loop_hook();
+
+        // Spin the internal loop until the main loop hook is set or no holds
+        // remain.
+        //
+        // If the main loop returns false we cannot guarantee the state of our
+        // promise queue (a module promise could be pending) so instead of
+        // draining the queue we instead just exit.
+        //
+        // Additional jobs could have been enqueued from the
+        // main loop hook
+        if (ok && !m_main_loop.spin(this)) {
+            exiting = true;
+        }
+    } while (ok && !exiting && m_main_loop_hook);
+
+    // 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.
+    // We do not drain if we are exiting.
+    if (!ok && !exiting) {
         JS::AutoSaveExceptionState saved_exc(m_cx);
         ok = run_jobs_fallible() && ok;
     }
diff --git a/gjs/mainloop.cpp b/gjs/mainloop.cpp
index 91109d61..b69d45a8 100644
--- a/gjs/mainloop.cpp
+++ b/gjs/mainloop.cpp
@@ -40,6 +40,8 @@ bool MainLoop::spin(GjsContextPrivate* gjs) {
             return false;
         }
     } while (
+        // and there is not a pending main loop hook
+        !gjs->has_main_loop_hook() &&
         // and there are pending sources or the job queue is not empty
         // continue spinning the event loop.
         (can_block() || !gjs->empty()));
diff --git a/gjs/promise.cpp b/gjs/promise.cpp
index 74127960..39939aa9 100644
--- a/gjs/promise.cpp
+++ b/gjs/promise.cpp
@@ -9,6 +9,7 @@
 #include <gio/gio.h>
 #include <glib-object.h>
 
+#include <js/CallAndConstruct.h>  // for JS::IsCallable
 #include <js/CallArgs.h>
 #include <js/PropertyAndElement.h>  // for JS_DefineFunctions
 #include <js/PropertySpec.h>
@@ -17,6 +18,7 @@
 #include <jsapi.h>  // for JS_NewPlainObject
 
 #include "gjs/context-private.h"
+#include "gjs/jsapi-util-args.h"
 #include "gjs/jsapi-util.h"
 #include "gjs/macros.h"
 #include "gjs/promise.h"
@@ -195,8 +197,36 @@ bool drain_microtask_queue(JSContext* cx, unsigned argc, JS::Value* vp) {
     return true;
 }
 
+GJS_JSAPI_RETURN_CONVENTION
+bool set_main_loop_hook(JSContext* cx, unsigned argc, JS::Value* vp) {
+    JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
+
+    JS::RootedObject callback(cx);
+    if (!gjs_parse_call_args(cx, "setMainLoopHook", args, "o", "callback",
+                             &callback)) {
+        return false;
+    }
+
+    if (!JS::IsCallable(callback)) {
+        gjs_throw(cx, "Main loop hook must be callable");
+        return false;
+    }
+
+    GjsContextPrivate* priv = GjsContextPrivate::from_cx(cx);
+    if (!priv->set_main_loop_hook(callback)) {
+        gjs_throw(
+            cx,
+            "A mainloop is already running. Did you already call runAsync()?");
+        return false;
+    }
+
+    args.rval().setUndefined();
+    return true;
+}
+
 JSFunctionSpec gjs_native_promise_module_funcs[] = {
-    JS_FN("drainMicrotaskQueue", &drain_microtask_queue, 0, 0), JS_FS_END};
+    JS_FN("drainMicrotaskQueue", &drain_microtask_queue, 0, 0),
+    JS_FN("setMainLoopHook", &set_main_loop_hook, 1, 0), JS_FS_END};
 
 bool gjs_define_native_promise_stuff(JSContext* cx,
                                      JS::MutableHandleObject module) {
diff --git a/installed-tests/js/.eslintrc.yml b/installed-tests/js/.eslintrc.yml
index a8d60f9a..74a8a4f3 100644
--- a/installed-tests/js/.eslintrc.yml
+++ b/installed-tests/js/.eslintrc.yml
@@ -25,7 +25,10 @@ rules:
 overrides:
   - files:
       - matchers.js
+      - minijasmine.js
+      - minijasmine-executor.js
       - testAsync.js
+      - testAsyncMainloop.js
       - testCairoModule.js
       - testConsole.js
       - testESModules.js
diff --git a/installed-tests/js/jsunit.gresources.xml b/installed-tests/js/jsunit.gresources.xml
index 537bfb2e..209274fc 100644
--- a/installed-tests/js/jsunit.gresources.xml
+++ b/installed-tests/js/jsunit.gresources.xml
@@ -7,6 +7,7 @@
     <file preprocess="xml-stripblanks">complex4.ui</file>
     <file>jasmine.js</file>
     <file>minijasmine.js</file>
+    <file>minijasmine-executor.js</file>
     <file>modules/alwaysThrows.js</file>
     <file>modules/badOverrides/GIMarshallingTests.js</file>
     <file>modules/badOverrides/Gio.js</file>
diff --git a/installed-tests/js/meson.build b/installed-tests/js/meson.build
index 7e8f4acb..5520a19d 100644
--- a/installed-tests/js/meson.build
+++ b/installed-tests/js/meson.build
@@ -240,6 +240,7 @@ modules_tests = [
     'Async',
     'Console',
     'ESModules',
+    'AsyncMainloop',
     'Encoding',
     'GLibLogWriter',
     'Global',
diff --git a/installed-tests/js/minijasmine-executor.js b/installed-tests/js/minijasmine-executor.js
new file mode 100644
index 00000000..3fe8b782
--- /dev/null
+++ b/installed-tests/js/minijasmine-executor.js
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: MIT OR LGPL-2.0-or-later
+// SPDX-FileCopyrightText: 2016 Philip Chimento <philip.chimento@gmail.com>
+// SPDX-FileCopyrightText: 2022 Evan Welsh <contact@evanwelsh.com>
+
+import * as system from 'system';
+
+import GLib from 'gi://GLib';
+import {environment, retval, errorsOutput, mainloop, mainloopLock} from './minijasmine.js';
+
+/* jasmineEnv.execute() queues up all the tests and runs them
+ * asynchronously. This should start after the main loop starts, otherwise
+ * we will hit the main loop only after several tests have already run. For
+ * consistency we should guarantee that there is a main loop running during
+ * all tests.
+ */
+GLib.idle_add(GLib.PRIORITY_DEFAULT, function () {
+    try {
+        environment.execute();
+    } catch (e) {
+        print('Bail out! Exception occurred inside Jasmine:', e);
+
+        mainloop.quit();
+
+        system.exit(1);
+    }
+
+    return GLib.SOURCE_REMOVE;
+});
+
+// Keep running the mainloop while mainloopLock
+// is not null and resolves true
+do {
+    // Run the mainloop
+
+    // This rule is to encourage parallelizing async
+    // operations, in this case we don't want that.
+    // eslint-disable-next-line no-await-in-loop
+    await mainloop.runAsync();
+// eslint-disable-next-line no-await-in-loop
+} while (await mainloopLock);
+
+// On exit, check the return value and print any errors which occurred
+if (retval !== 0) {
+    printerr(errorsOutput.join('\n'));
+    print('# Test script failed; see test log for assertions');
+    system.exit(retval);
+}
diff --git a/installed-tests/js/minijasmine.js b/installed-tests/js/minijasmine.js
index 076958f8..36f953bc 100644
--- a/installed-tests/js/minijasmine.js
+++ b/installed-tests/js/minijasmine.js
@@ -1,8 +1,8 @@
-#!/usr/bin/env gjs
+#!/usr/bin/env -S gjs -m
 // SPDX-License-Identifier: MIT OR LGPL-2.0-or-later
 // SPDX-FileCopyrightText: 2016 Philip Chimento <philip.chimento@gmail.com>
 
-const GLib = imports.gi.GLib;
+import GLib from 'gi://GLib';
 
 function _filterStack(stack) {
     if (!stack)
@@ -16,16 +16,17 @@ function _filterStack(stack) {
 
 let jasmineRequire = imports.jasmine.getJasmineRequireObj();
 let jasmineCore = jasmineRequire.core(jasmineRequire);
-globalThis._jasmineEnv = jasmineCore.getEnv();
-globalThis._jasmineEnv.configure({
+
+export let environment = jasmineCore.getEnv();
+environment.configure({
     random: false,
 });
-globalThis._jasmineMain = GLib.MainLoop.new(null, false);
-globalThis._jasmineRetval = 0;
-globalThis._jasmineErrorsOutput = [];
+export const mainloop = GLib.MainLoop.new(null, false);
+export let retval = 0;
+export let errorsOutput = [];
 
 // Install Jasmine API on the global object
-let jasmineInterface = jasmineRequire.interface(jasmineCore, globalThis._jasmineEnv);
+let jasmineInterface = jasmineRequire.interface(jasmineCore, environment);
 Object.assign(globalThis, jasmineInterface);
 
 // Reporter that outputs according to the Test Anything Protocol
@@ -48,7 +49,7 @@ class TapReporter {
             });
         });
 
-        globalThis._jasmineMain.quit();
+        mainloop.quit();
     }
 
     suiteDone(result) {
@@ -92,20 +93,20 @@ class TapReporter {
                 let stackTrace = _filterStack(failedExpectation.stack).trim();
                 output.push(...stackTrace.split('\n').map(str => `  ${str}`));
 
-                if (globalThis._jasmineErrorsOutput.length) {
-                    globalThis._jasmineErrorsOutput.push(
+                if (errorsOutput.length) {
+                    errorsOutput.push(
                         Array(GLib.getenv('COLUMNS') || 80).fill('―').join(''));
                 }
 
-                globalThis._jasmineErrorsOutput.push(`Test: ${result.fullName}`);
-                globalThis._jasmineErrorsOutput.push(...output);
+                errorsOutput.push(`Test: ${result.fullName}`);
+                errorsOutput.push(...output);
                 print(output.map(l => `# ${l}`).join('\n'));
             });
         }
     }
 }
 
-globalThis._jasmineEnv.addReporter(new TapReporter());
+environment.addReporter(new TapReporter());
 
 // If we're running the tests in certain JS_GC_ZEAL modes or Valgrind, then some
 // will time out if the CI machine is under a certain load. In that case
@@ -114,3 +115,30 @@ const gcZeal = GLib.getenv('JS_GC_ZEAL');
 const valgrind = GLib.getenv('VALGRIND');
 if (valgrind || (gcZeal && (gcZeal === '2' || gcZeal.startsWith('2,') || gcZeal === '4')))
     jasmine.DEFAULT_TIMEOUT_INTERVAL *= 5;
+
+/**
+ * The Promise (or null) that minijasmine-executor locks on
+ * to avoid exiting prematurely
+ */
+export let mainloopLock = null;
+
+/**
+ * Stops the mainloop but prevents the minijasmine-executor from
+ * exiting.
+ *
+ * @returns a callback which returns control to minijasmine-executor
+ */
+export function acquireMainloop() {
+    let resolve;
+    mainloopLock = new Promise(_resolve => (resolve = _resolve));
+
+    if (!mainloop.is_running())
+        throw new Error("Main loop was stopped already, can't acquire");
+
+    mainloop.quit();
+
+    return () => {
+        mainloopLock = null;
+        resolve(true);
+    };
+}
diff --git a/installed-tests/js/testAsyncMainloop.js b/installed-tests/js/testAsyncMainloop.js
new file mode 100644
index 00000000..ec36a3a2
--- /dev/null
+++ b/installed-tests/js/testAsyncMainloop.js
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: MIT OR LGPL-2.0-or-later
+// SPDX-FileCopyrightText: 2022 Evan Welsh <contact@evanwelsh.com>
+
+import GLib from 'gi://GLib';
+import {acquireMainloop} from 'resource:///org/gjs/jsunit/minijasmine.js';
+
+describe('Async mainloop', function () {
+    let loop, quit;
+
+    beforeEach(function () {
+        loop = new GLib.MainLoop(null, false);
+        quit = jasmine.createSpy('quit').and.callFake(() => {
+            loop.quit();
+            return GLib.SOURCE_REMOVE;
+        });
+    });
+
+    it('resolves when main loop exits', async function () {
+        const release = acquireMainloop();
+
+        GLib.timeout_add(GLib.PRIORITY_DEFAULT, 50, quit);
+
+        await loop.runAsync();
+
+        expect(quit).toHaveBeenCalled();
+
+        release();
+    });
+});
diff --git a/installed-tests/minijasmine.cpp b/installed-tests/minijasmine.cpp
index 2e92d640..9d1c65f9 100644
--- a/installed-tests/minijasmine.cpp
+++ b/installed-tests/minijasmine.cpp
@@ -69,16 +69,16 @@ main(int argc, char **argv)
 
     GError *error = NULL;
     bool success;
-    int code;
-
+    uint8_t code;
+    uint8_t u8_exitcode_ignored;
     int exitcode_ignored;
-    if (!gjs_context_eval(cx, "imports.minijasmine;", -1, "<jasmine>",
-                          &exitcode_ignored, &error))
+    if (!gjs_context_eval_module_file(
+            cx, "resource:///org/gjs/jsunit/minijasmine.js",
+            &u8_exitcode_ignored, &error))
         bail_out(cx, error);
 
     bool eval_as_module = argc >= 3 && strcmp(argv[2], "-m") == 0;
     if (eval_as_module) {
-        uint8_t u8_exitcode_ignored;
         success = gjs_context_eval_module_file(cx, argv[1],
                                                &u8_exitcode_ignored, &error);
     } else {
@@ -87,43 +87,12 @@ main(int argc, char **argv)
     if (!success)
         bail_out(cx, error);
 
-    /* jasmineEnv.execute() queues up all the tests and runs them
-     * asynchronously. This should start after the main loop starts, otherwise
-     * we will hit the main loop only after several tests have already run. For
-     * consistency we should guarantee that there is a main loop running during
-     * all tests. */
-    const char *start_suite_script =
-        "const GLib = imports.gi.GLib;\n"
-        "GLib.idle_add(GLib.PRIORITY_DEFAULT, function () {\n"
-        "    try {\n"
-        "        window._jasmineEnv.execute();\n"
-        "    } catch (e) {\n"
-        "        print('Bail out! Exception occurred inside Jasmine:', e);\n"
-        "        window._jasmineRetval = 1;\n"
-        "        window._jasmineMain.quit();\n"
-        "    }\n"
-        "    return GLib.SOURCE_REMOVE;\n"
-        "});\n"
-        "window._jasmineMain.run();\n"
-        "window._jasmineRetval;";
-    success = gjs_context_eval(cx, start_suite_script, -1, "<jasmine-start>",
-                               &code, &error);
+    success = gjs_context_eval_module_file(
+        cx, "resource:///org/gjs/jsunit/minijasmine-executor.js", &code,
+        &error);
     if (!success)
         bail_out(cx, error);
 
-    if (code != 0) {
-        success = gjs_context_eval(cx, R"js(
-            printerr(globalThis._jasmineErrorsOutput.join('\n'));
-        )js",
-                                   -1, "<jasmine-error-logs>", &code, &error);
-
-        if (!success)
-            bail_out(cx, error->message);
-
-        if (code != 0)
-            g_print("# Test script failed; see test log for assertions\n");
-    }
-
     if (coverage) {
         gjs_coverage_write_statistics(coverage);
         g_clear_object(&coverage);
diff --git a/modules/core/overrides/GLib.js b/modules/core/overrides/GLib.js
index e2fa58dc..023cf795 100644
--- a/modules/core/overrides/GLib.js
+++ b/modules/core/overrides/GLib.js
@@ -2,6 +2,7 @@
 // SPDX-FileCopyrightText: 2011 Giovanni Campagna
 
 const ByteArray = imports.byteArray;
+const {setMainLoopHook} = imports._promiseNative;
 
 let GLib;
 
@@ -261,6 +262,18 @@ function _init() {
 
     GLib = this;
 
+    GLib.MainLoop.prototype.runAsync = function (...args) {
+        return new Promise((resolve, reject) => {
+            setMainLoopHook(() => {
+                try {
+                    resolve(this.run(...args));
+                } catch (error) {
+                    reject(error);
+                }
+            });
+        });
+    };
+
     // For convenience in property min or max values, since GLib.MAXINT64 and
     // friends will log a warning when used
     this.MAXINT64_BIGINT = 0x7fff_ffff_ffff_ffffn;
diff --git a/modules/core/overrides/Gio.js b/modules/core/overrides/Gio.js
index 94fa8188..66f36d96 100644
--- a/modules/core/overrides/Gio.js
+++ b/modules/core/overrides/Gio.js
@@ -464,6 +464,8 @@ function _warnNotIntrospectable(funcName, replacement) {
 function _init() {
     Gio = this;
 
+    Gio.Application.prototype.runAsync = GLib.MainLoop.prototype.runAsync;
+
     Gio.DBus = {
         // Namespace some functions
         get: Gio.bus_get,