context: Fix job queue ending prematurely when dispatcher reset

When the promise job dispatcher is reset, its GCancellable is replaced
with a different instance. However, the original GCancellable instance was
passed to GjsContextPrivate::runJobs(), which aborts the job queue if the
GCancellable is cancelled.

This wasn't normally a problem, because the GCancellable isn't usually
cancelled. However, running JS with code coverage instrumentation starts
and stops the job dispatcher after every job, in order for the debugger
code to do its thing. So the GCancellable was getting cancelled and
immediately replaced with a fresh one, but the job queue was still getting
aborted.

This still didn't seem to have any noticeable effect until we merged the
runAsync() feature recently. From that point, async module evaluate
operations started getting dropped on the floor when code coverage was
active, which is very bad news.

The fix is to not hold on to the GCancellable across jobs. Instead of
checking whether the GCancellable is cancelled, instead check whether the
promise job dispatcher is running.

3 files changed, 6 insertions, 15 deletions

diff --git a/gjs/context-private.h b/gjs/context-private.h
index d8b7b85d..147feae4 100644
--- a/gjs/context-private.h
+++ b/gjs/context-private.h
@@ -17,7 +17,6 @@
 #include <utility>  // for pair
 #include <vector>
 
-#include <gio/gio.h>
 #include <glib-object.h>
 #include <glib.h>
 
@@ -248,13 +247,11 @@ class GjsContextPrivate : public JS::JobQueue {
                            JS::HandleObject allocation_site,
                            JS::HandleObject incumbent_global) override;
     void runJobs(JSContext* cx) override;
-    void runJobs(JSContext* cx, GCancellable* cancellable);
     [[nodiscard]] bool empty() const override { return m_job_queue.empty(); }
     js::UniquePtr<JS::JobQueue::SavedJobQueue> saveJobQueue(
         JSContext* cx) override;
 
-    GJS_JSAPI_RETURN_CONVENTION bool run_jobs_fallible(
-        GCancellable* cancellable = nullptr);
+    GJS_JSAPI_RETURN_CONVENTION bool run_jobs_fallible();
     void register_unhandled_promise_rejection(uint64_t id, GjsAutoChar&& stack);
     void unregister_unhandled_promise_rejection(uint64_t id);
     void warn_about_unhandled_promise_rejections();
diff --git a/gjs/context.cpp b/gjs/context.cpp
index 8d2cbc6e..59ac8afd 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -1045,12 +1045,10 @@ bool GjsContextPrivate::enqueuePromiseJob(JSContext* cx [[maybe_unused]],
 
 // Override of JobQueue::runJobs(). Called by js::RunJobs(), and when execution
 // of the job queue was interrupted by the debugger and is resuming.
-void GjsContextPrivate::runJobs(JSContext* cx) { runJobs(cx, nullptr); }
-
-void GjsContextPrivate::runJobs(JSContext* cx, GCancellable* cancellable) {
+void GjsContextPrivate::runJobs(JSContext* cx) {
     g_assert(cx == m_cx);
     g_assert(from_cx(cx) == this);
-    if (!run_jobs_fallible(cancellable))
+    if (!run_jobs_fallible())
         gjs_log_exception(cx);
 }
 
@@ -1066,7 +1064,7 @@ void GjsContextPrivate::runJobs(JSContext* cx, GCancellable* cancellable) {
  * Returns: false if one of the jobs threw an uncatchable exception;
  * otherwise true.
  */
-bool GjsContextPrivate::run_jobs_fallible(GCancellable* cancellable) {
+bool GjsContextPrivate::run_jobs_fallible() {
     bool retval = true;
 
     if (m_draining_job_queue || m_should_exit)
@@ -1083,7 +1081,7 @@ bool GjsContextPrivate::run_jobs_fallible(GCancellable* cancellable) {
      * it's crucial to recheck the queue length during each iteration. */
     for (size_t ix = 0; ix < m_job_queue.length(); ix++) {
         /* A previous job might have set this flag. e.g., System.exit(). */
-        if (m_should_exit || g_cancellable_is_cancelled(cancellable)) {
+        if (m_should_exit || !m_dispatcher.is_running()) {
             gjs_debug(GJS_DEBUG_MAINLOOP, "Stopping jobs because of %s",
                       m_should_exit ? "exit" : "main loop cancel");
             break;
diff --git a/gjs/promise.cpp b/gjs/promise.cpp
index 0507eaf3..f248b019 100644
--- a/gjs/promise.cpp
+++ b/gjs/promise.cpp
@@ -82,12 +82,8 @@ class PromiseJobDispatcher::Source : public GSource {
         // next one to execute. (it will starve the other sources)
         g_source_set_ready_time(this, -1);
 
-        // A reference to the current cancellable is needed in case any
-        // jobs reset PromiseJobDispatcher and thus replace the cancellable.
-        GjsAutoUnref<GCancellable> cancellable(m_cancellable,
-                                               GjsAutoTakeOwnership{});
         // Drain the job queue.
-        m_gjs->runJobs(m_gjs->context(), cancellable);
+        m_gjs->runJobs(m_gjs->context());
 
         return G_SOURCE_CONTINUE;
     }