worker: make terminate() abort a worker running synchronous JS

src/private.h

@@ -35,6 +35,7 @@
 #ifdef TJS_HAVE_SQLITE
 #include <sqlite3.h>
 #endif
+#include <stdatomic.h>
 #include <stdbool.h>
 #ifndef _WIN32
 #include <unistd.h>
@@ -149,6 +150,9 @@ struct TJSRuntime {
     } jobs;
     uv_async_t stop;
     bool is_worker;
+    /* Set by the parent thread when it reaps the worker; read by the worker's
+     * interrupt handler to abort JS that never yields to the loop. */
+    atomic_bool terminating;
     bool draining_microtasks;
     bool microtasks_stopped;
 #ifdef TJS_HAVE_WASM
@@ -342,6 +346,9 @@ void tjs__lws_format_host(char *buf, size_t buflen, const char *scheme, const ch
 void tjs__lws_request_path(const char *url, char *buf, size_t buflen);
 
 uv_loop_t *TJS_GetLoop(TJSRuntime *qrt);
+/* Stop a worker runtime from another thread, aborting any JS it is running
+ * (unlike TJS_Stop, which waits for the running JS to return to the loop). */
+void tjs__runtime_terminate(TJSRuntime *qrt);
 TJSRuntime *TJS_NewRuntimeWorker(void);
 TJSRuntime *TJS_NewRuntimeInternal(bool is_worker, TJSRunOptions *options);
 JSValue TJS_EvalScript(JSContext *ctx, const char *filename);

src/vm.c

@@ -342,6 +342,12 @@ void tjs__worker_handle_uncaught(JSContext *ctx, JSValueConst exc) {
         return;
     }
 
+    /* Errors raised while being terminated (chiefly the interrupt that aborts
+     * the running JS) are not reported. */
+    if (atomic_load(&qrt->terminating)) {
+        return;
+    }
+
     /* Dispatch a cancelable 'error' ErrorEvent at the worker global scope so
      * self.onerror can observe (and, via preventDefault(), suppress propagation
      * to) the parent. */
@@ -436,6 +442,12 @@ static void uv__stop(uv_async_t *handle) {
     uv_stop(&qrt->loop);
 }
 
+/* Runs on the worker thread, polled by QuickJS while JS executes. */
+static int tjs__worker_interrupt_handler(JSContext *ctx, void *opaque) {
+    TJSRuntime *qrt = opaque;
+    return atomic_load(&qrt->terminating);
+}
+
 void TJS_DefaultOptions(TJSRunOptions *options) {
     static TJSRunOptions default_options = { .mem_limit = 0, .stack_size = TJS__DEFAULT_STACK_SIZE };
 
@@ -528,6 +540,10 @@ TJSRuntime *TJS_NewRuntimeInternal(bool is_worker, TJSRunOptions *options) {
     /* Worker support */
     qrt->is_worker = is_worker;
     JS_SetCanBlock(rt, is_worker);
+    atomic_init(&qrt->terminating, false);
+    if (is_worker) {
+        JS_SetInterruptHandler(rt, tjs__worker_interrupt_handler, qrt);
+    }
 
     CHECK_EQ(uv_loop_init(&qrt->loop), 0);
 
@@ -1022,6 +1038,10 @@ void tjs__drain_microtasks(JSContext *ctx) {
     }
 }
 
+static bool tjs__worker_terminating(TJSRuntime *qrt) {
+    return qrt->is_worker && atomic_load(&qrt->terminating);
+}
+
 void tjs__execute_jobs(JSContext *ctx) {
     tjs__drain_microtasks(ctx);
 
@@ -1035,6 +1055,17 @@ void tjs__execute_jobs(JSContext *ctx) {
     list_for_each_safe(el, el1, &qrt->pending_rejections) {
         TJSPendingRejection *pr = list_entry(el, TJSPendingRejection, link);
 
+        /* A terminated worker drops its rejections. Module evaluation turns the
+         * interrupt that aborts a busy module body into a rejection of the module
+         * promise, so this is where the abort itself usually shows up. */
+        if (tjs__worker_terminating(qrt)) {
+            list_del(&pr->link);
+            JS_FreeValue(ctx, pr->promise);
+            JS_FreeValue(ctx, pr->reason);
+            js_free(ctx, pr);
+            continue;
+        }
+
         JSValue event_name = JS_NewString(ctx, "unhandledrejection");
         JSValue args[3];
         args[0] = event_name;
@@ -1049,14 +1080,20 @@ void tjs__execute_jobs(JSContext *ctx) {
         /* User code can break the event constructor (e.g. by overwriting
          * globals it uses), so treat that as an unhandled rejection. */
         if (JS_IsException(event)) {
-            tjs_dump_error(ctx);
+            if (!tjs__worker_terminating(qrt)) {
+                tjs_dump_error(ctx);
+            }
             should_abort = true;
         } else {
             JSValue ret = tjs__dispatch_event(ctx, &event);
             JS_FreeValue(ctx, event);
 
             if (JS_IsException(ret)) {
-                tjs_dump_error(ctx);
+                /* Terminated mid-dispatch: the pending interrupt is replaced
+                 * below and freed silently by TJS_Run. */
+                if (!tjs__worker_terminating(qrt)) {
+                    tjs_dump_error(ctx);
+                }
                 should_abort = true;
             } else if (JS_ToBool(ctx, ret)) {
                 /* The event wasn't cancelled, abort. */
@@ -1182,8 +1219,13 @@ int TJS_Run(TJSRuntime *qrt) {
     }
 
     if (JS_HasException(qrt->ctx)) {
-        tjs_dump_error(qrt->ctx);
-        ret = 1;
+        if (tjs__worker_terminating(qrt)) {
+            /* A terminated worker exits silently. */
+            JS_FreeValue(qrt->ctx, JS_GetException(qrt->ctx));
+        } else {
+            tjs_dump_error(qrt->ctx);
+            ret = 1;
+        }
     }
 
     return ret;
@@ -1198,6 +1240,13 @@ void TJS_Stop(TJSRuntime *qrt) {
     }
 }
 
+void tjs__runtime_terminate(TJSRuntime *qrt) {
+    CHECK_NOT_NULL(qrt);
+    /* Set before stopping, so the worker sees the flag no later than the stop. */
+    atomic_store(&qrt->terminating, true);
+    TJS_Stop(qrt);
+}
+
 uv_loop_t *TJS_GetLoop(TJSRuntime *qrt) {
     return &qrt->loop;
 }

src/worker.c

@@ -49,6 +49,11 @@ void tjs__worker_post_error(JSContext *ctx, JSValueConst error) {
         return;
     }
 
+    /* The parent is terminating us: it no longer wants to hear about errors. */
+    if (atomic_load(&qrt->terminating)) {
+        return;
+    }
+
     JSValue obj = JS_NewObject(ctx);
     if (JS_IsException(obj)) {
         JS_FreeValue(ctx, JS_GetException(ctx));
@@ -229,8 +234,13 @@ static void worker_entry(void *arg) {
  * the worker is still running or has already stopped itself. */
 static void worker_reap(TJSWorker *w) {
     if (w->wrt) {
-        TJS_Stop(w->wrt);
+        /* Aborts JS that never returns to the worker's loop (a busy loop). */
+        tjs__runtime_terminate(w->wrt);
         CHECK_EQ(uv_thread_join(&w->tid), 0);
+        /* TJS_FreeRuntime can run JS on this thread (the handle drain). Without
+         * the handler that JS would be interrupted midway, and an error path
+         * would TJS_Stop the already-closed `stop` handle. */
+        JS_SetInterruptHandler(w->wrt->rt, NULL, NULL);
         TJS_FreeRuntime(w->wrt);
         w->wrt = NULL;
     }

tests/helpers/worker-terminate-busy.js

@@ -0,0 +1,35 @@
+// Terminates (or, with "parent-throw", abandons via an uncaught error) a worker
+// that is stuck in a synchronous busy loop. The mode picks where the loop runs.
+const mode = tjs.args.at(-1);
+
+const prelude = 'self.onerror = e => console.error(\'worker onerror\', e.message);'
+    + 'self.onunhandledrejection = e => console.error(\'worker onunhandledrejection\', e.reason?.message);';
+const sources = {
+    'top-level': 'self.postMessage(\'busy\'); for (;;) {}',
+    'onmessage': 'self.onmessage = () => { self.postMessage(\'busy\'); for (;;) {} };',
+    'await': 'await new Promise(r => setTimeout(r, 0)); self.postMessage(\'busy\'); for (;;) {}',
+    'parent-throw': 'self.postMessage(\'busy\'); for (;;) {}',
+};
+
+const url = URL.createObjectURL(new Blob([ prelude + sources[mode] ], { type: 'text/javascript' }));
+const w = new Worker(url);
+
+URL.revokeObjectURL(url);
+
+w.onerror = e => console.log('error event', e.message);
+
+if (mode === 'parent-throw') {
+    // Fatal (unlike a throw in an event listener, which is only reported), so
+    // the parent tears down with the worker still busy.
+    await new Promise(resolve => w.onmessage = resolve);
+    throw new Error('boom');
+}
+
+w.onmessage = () => {
+    w.terminate();
+    console.log('terminate returned');
+};
+
+if (mode === 'onmessage') {
+    w.postMessage('go');
+}

tests/test-worker-terminate-busy.js

@@ -0,0 +1,39 @@
+// terminate() aborts a worker stuck in synchronous JS instead of blocking the
+// parent forever, and the abort is silent: no error event and nothing on stderr.
+// An uncaught error in the parent tears a busy worker down the same way.
+import assert from 'tjs:assert';
+import path from 'tjs:path';
+
+async function runHelper(mode) {
+    const args = [ tjs.exePath, 'run', path.join(import.meta.dirname, 'helpers', 'worker-terminate-busy.js'), mode ];
+    const proc = tjs.spawn(args, { stdout: 'pipe', stderr: 'pipe' });
+    let timedOut = false;
+    // A hang fails here rather than at the runner's timeout; GC-stress builds are far slower.
+    const watchdog = setTimeout(() => {
+        timedOut = true;
+        proc.kill('SIGKILL');
+    }, tjs.env.TJS_GC_STRESS ? 60000 : 10000);
+    const [ status, stdout, stderr ] = await Promise.all([ proc.wait(), proc.stdout.text(), proc.stderr.text() ]);
+
+    clearTimeout(watchdog);
+    assert.ok(!timedOut, `${mode}: parent exited (did not hang)`);
+
+    return { status, stdout: stdout.trim(), stderr };
+}
+
+for (const mode of [ 'top-level', 'onmessage', 'await' ]) {
+    const { status, stdout, stderr } = await runHelper(mode);
+
+    assert.eq(status.exit_status, 0, `${mode}: exits cleanly`);
+    assert.eq(stdout, 'terminate returned', `${mode}: terminate() returned, no error event`);
+    assert.eq(stderr, '', `${mode}: nothing on stderr`);
+}
+
+{
+    const { status, stdout, stderr } = await runHelper('parent-throw');
+
+    assert.eq(status.exit_status, 1, 'parent-throw: exits with the error status');
+    assert.eq(stdout, '', 'parent-throw: no error event');
+    assert.ok(stderr.includes('boom'), 'parent-throw: parent error reported');
+    assert.ok(!stderr.includes('interrupted'), 'parent-throw: worker abort is silent');
+}