Skip to content

Rework promise rejection tracker #1058

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 20, 2025
Merged

Rework promise rejection tracker #1058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 77 additions & 4 deletions quickjs-libc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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 {
Expand Down
61 changes: 9 additions & 52 deletions quickjs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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];
Expand All @@ -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,
Expand Down Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is is_handled==true? I would've expected false.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because here we are attaching a catch AFAIK.

}
i = s->promise_state - JS_PROMISE_FULFILLED;
rd = rd_array[i];
Expand Down