Rework promise rejection tracker

This reverts a75498b and
a4a5a17 (except the tests) and adapts
bellard/quickjs@3c39307
which also passes the tests and feels like a better solution overall.

Ref: #1038

Author: saghul

quickjs-libc.c

@@ -128,6 +128,12 @@ typedef struct {
     JSValue func;
 } JSOSTimer;
 
+typedef struct {
+    struct list_head link;
+    JSValue promise;
+    JSValue reason;
+} JSRejectedPromiseEntry;
+
 #ifdef USE_WORKER
 
 typedef struct {
@@ -168,6 +174,7 @@ typedef struct JSThreadState {
     struct list_head os_signal_handlers; /* list JSOSSignalHandler.link */
     struct list_head os_timers; /* list of JSOSTimer.link */
     struct list_head port_list; /* list of JSWorkerMessageHandler.link */
+    struct list_head rejected_promise_list; /* list of JSRejectedPromiseEntry.link */
     int eval_script_recurse; /* only used in the main thread */
     int64_t next_timer_id; /* for setTimeout / setInterval */
     bool can_js_os_poll;
@@ -4280,6 +4287,7 @@ void js_std_init_handlers(JSRuntime *rt)
     init_list_head(&ts->os_signal_handlers);
     init_list_head(&ts->os_timers);
     init_list_head(&ts->port_list);
+    init_list_head(&ts->rejected_promise_list);
 
     ts->next_timer_id = 1;
 
@@ -4299,6 +4307,14 @@ void js_std_init_handlers(JSRuntime *rt)
 #endif
 }
 
+static void free_rp(JSRuntime *rt, JSRejectedPromiseEntry *rp)
+{
+    list_del(&rp->link);
+    JS_FreeValueRT(rt, rp->promise);
+    JS_FreeValueRT(rt, rp->reason);
+    js_free_rt(rt, rp);
+}
+
 void js_std_free_handlers(JSRuntime *rt)
 {
     JSThreadState *ts = js_get_thread_state(rt);
@@ -4319,6 +4335,11 @@ void js_std_free_handlers(JSRuntime *rt)
         free_timer(rt, th);
     }
 
+    list_for_each_safe(el, el1, &ts->rejected_promise_list) {
+        JSRejectedPromiseEntry *rp = list_entry(el, JSRejectedPromiseEntry, link);
+        free_rp(rt, rp);
+    }
+
 #ifdef USE_WORKER
     /* XXX: free port_list ? */
     js_free_message_pipe(ts->recv_pipe);
@@ -4366,14 +4387,62 @@ void js_std_dump_error(JSContext *ctx)
     JS_FreeValue(ctx, exception_val);
 }
 
+static JSRejectedPromiseEntry *find_rejected_promise(JSContext *ctx, JSThreadState *ts,
+                                                     JSValueConst promise)
+{
+    struct list_head *el;
+
+    list_for_each(el, &ts->rejected_promise_list) {
+        JSRejectedPromiseEntry *rp = list_entry(el, JSRejectedPromiseEntry, link);
+        if (JS_IsSameValue(ctx, rp->promise, promise))
+            return rp;
+    }
+    return NULL;
+}
+
 void js_std_promise_rejection_tracker(JSContext *ctx, JSValueConst promise,
                                       JSValueConst reason,
                                       bool is_handled, void *opaque)
 {
-    if (!is_handled) {
-        fprintf(stderr, "Possibly unhandled promise rejection: ");
-        js_std_dump_error1(ctx, reason);
-        fflush(stderr);
+
+    JSRuntime *rt = JS_GetRuntime(ctx);
+    JSThreadState *ts = js_get_thread_state(rt);
+    JSRejectedPromiseEntry *rp = find_rejected_promise(ctx, ts, promise);
+
+    if (is_handled) {
+        /* the rejection is handled, so the entry can be removed if present */
+        if (rp)
+            free_rp(rt, rp);
+    } else {
+        /* add a new entry if needed */
+        if (!rp) {
+            rp = js_malloc_rt(rt, sizeof(*rp));
+            if (rp) {
+                rp->promise = JS_DupValue(ctx, promise);
+                rp->reason = JS_DupValue(ctx, reason);
+                list_add_tail(&rp->link, &ts->rejected_promise_list);
+            }
+        }
+    }
+}
+
+/* check if there are pending promise rejections. It must be done
+   asynchrously in case a rejected promise is handled later. Currently
+   we do it once the application is about to sleep. It could be done
+   more often if needed. */
+static void js_std_promise_rejection_check(JSContext *ctx)
+{
+    JSRuntime *rt = JS_GetRuntime(ctx);
+    JSThreadState *ts = js_get_thread_state(rt);
+    struct list_head *el;
+
+    if (unlikely(!list_empty(&ts->rejected_promise_list))) {
+        list_for_each(el, &ts->rejected_promise_list) {
+            JSRejectedPromiseEntry *rp = list_entry(el, JSRejectedPromiseEntry, link);
+            fprintf(stderr, "Possibly unhandled promise rejection: ");
+            js_std_dump_error1(ctx, rp->reason);
+            fflush(stderr);
+        }
         exit(1);
     }
 }
@@ -4397,6 +4466,8 @@ int js_std_loop(JSContext *ctx)
             }
         }
 
+        js_std_promise_rejection_check(ctx);
+
         if (!ts->can_js_os_poll || js_os_poll(ctx))
             break;
     }
@@ -4431,6 +4502,8 @@ JSValue js_std_await(JSContext *ctx, JSValue obj)
             if (err < 0) {
                 js_std_dump_error(ctx1);
             }
+            if (err == 0)
+                js_std_promise_rejection_check(ctx);
             if (ts->can_js_os_poll)
                 js_os_poll(ctx);
         } else {

quickjs.c

@@ -50208,51 +50208,6 @@ static JSValue promise_reaction_job(JSContext *ctx, int argc,
     return res2;
 }
 
-static JSValue promise_rejection_tracker_job(JSContext *ctx, int argc,
-                                             JSValueConst *argv)
-{
-    JSRuntime *rt;
-    JSPromiseData *s;
-    JSValueConst promise;
-    struct list_head *el, *el1;
-    JSJobEntry *job;
-    bool has_other_promise_jobs;
-
-    assert(argc == 1);
-
-    rt = ctx->rt;
-    promise = argv[0];
-    s = JS_GetOpaque(promise, JS_CLASS_PROMISE);
-
-    if (!s || s->promise_state != JS_PROMISE_REJECTED)
-        return JS_UNDEFINED; /* should never happen */
-
-    promise_trace(ctx, "promise_rejection_tracker_job\n");
-
-    // Push the rejection tracker jobs to the end of the queue if there are other jobs.
-    // This allows us to handle rejections that get added later and thus would handle the
-    // rejection _after_ we check for it.
-    has_other_promise_jobs = false;
-    list_for_each_safe(el, el1, &rt->job_list) {
-        job = list_entry(el, JSJobEntry, link);
-        if (job->job_func == promise_reaction_job || job->job_func == js_promise_resolve_thenable_job) {
-            has_other_promise_jobs = true;
-            break;
-        }
-    }
-    if (has_other_promise_jobs) {
-        JS_EnqueueJob(ctx, promise_rejection_tracker_job, 1, &promise);
-        return JS_UNDEFINED;
-    }
-
-    // Check again in case the hook was removed.
-    if (rt->host_promise_rejection_tracker)
-        rt->host_promise_rejection_tracker(
-            ctx, promise, s->promise_result, s->is_handled, rt->host_promise_rejection_tracker_opaque);
-
-    return JS_UNDEFINED;
-}
-
 void JS_SetPromiseHook(JSRuntime *rt, JSPromiseHook promise_hook, void *opaque)
 {
     rt->promise_hook = promise_hook;
@@ -50290,6 +50245,13 @@ static void fulfill_or_reject_promise(JSContext *ctx, JSValueConst promise,
         }
     }
 
+    if (s->promise_state == JS_PROMISE_REJECTED && !s->is_handled) {
+        JSRuntime *rt = ctx->rt;
+        if (rt->host_promise_rejection_tracker)
+            rt->host_promise_rejection_tracker(ctx, promise, value, false,
+                                               rt->host_promise_rejection_tracker_opaque);
+    }
+
     list_for_each_safe(el, el1, &s->promise_reactions[is_reject]) {
         rd = list_entry(el, JSPromiseReactionData, link);
         args[0] = rd->resolving_funcs[0];
@@ -50307,12 +50269,6 @@ static void fulfill_or_reject_promise(JSContext *ctx, JSValueConst promise,
         list_del(&rd->link);
         promise_reaction_data_free(ctx->rt, rd);
     }
-
-    if (s->promise_state == JS_PROMISE_REJECTED && !s->is_handled) {
-        JSRuntime *rt = ctx->rt;
-        if (rt->host_promise_rejection_tracker)
-            JS_EnqueueJob(ctx, promise_rejection_tracker_job, 1, &promise);
-    }
 }
 
 static JSValue js_promise_resolve_thenable_job(JSContext *ctx,
@@ -51060,7 +51016,8 @@ static __exception int perform_promise_then(JSContext *ctx,
         if (s->promise_state == JS_PROMISE_REJECTED && !s->is_handled) {
             JSRuntime *rt = ctx->rt;
             if (rt->host_promise_rejection_tracker)
-                JS_EnqueueJob(ctx, promise_rejection_tracker_job, 1, &promise);
+                rt->host_promise_rejection_tracker(ctx, promise, s->promise_result,
+                                                   true, rt->host_promise_rejection_tracker_opaque);
         }
         i = s->promise_state - JS_PROMISE_FULFILLED;
         rd = rd_array[i];