From 8c43cf9bc3f3d4fae45e39708a26ed33f0c28b7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C3=BAl=20Ibarra=20Corretg=C3=A9?= Date: Tue, 6 May 2025 10:14:53 +0200 Subject: [PATCH 1/2] Fix unhandled promise rejection tracker, again Delay checking for unhandled rejections so we can make sure to check after other promise jobs have ran. --- quickjs.c | 19 +++++++++++++++++++ tests/{bug39.js => bug39/1.js} | 0 tests/bug39/2.js | 6 ++++++ tests/bug39/3.js | 7 +++++++ 4 files changed, 32 insertions(+) rename tests/{bug39.js => bug39/1.js} (100%) create mode 100644 tests/bug39/2.js create mode 100644 tests/bug39/3.js diff --git a/quickjs.c b/quickjs.c index b6277b562..a26cd5dad 100644 --- a/quickjs.c +++ b/quickjs.c @@ -50221,6 +50221,9 @@ static JSValue promise_rejection_tracker_job(JSContext *ctx, int argc, JSRuntime *rt; JSPromiseData *s; JSValueConst promise; + struct list_head *el, *el1; + JSJobEntry *job; + bool has_other_jobs; assert(argc == 1); @@ -50233,6 +50236,22 @@ static JSValue promise_rejection_tracker_job(JSContext *ctx, int argc, 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_jobs = false; + list_for_each_safe(el, el1, &rt->job_list) { + job = list_entry(el, JSJobEntry, link); + if (job->job_func != promise_rejection_tracker_job) { + has_other_jobs = true; + break; + } + } + if (has_other_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( diff --git a/tests/bug39.js b/tests/bug39/1.js similarity index 100% rename from tests/bug39.js rename to tests/bug39/1.js diff --git a/tests/bug39/2.js b/tests/bug39/2.js new file mode 100644 index 000000000..a786a7742 --- /dev/null +++ b/tests/bug39/2.js @@ -0,0 +1,6 @@ +/*--- +flags: [qjs:track-promise-rejections] +---*/ + +const error = await Promise.resolve().then(() => Promise.reject('reject')).catch(e => e) +print('Got this error:', error) diff --git a/tests/bug39/3.js b/tests/bug39/3.js new file mode 100644 index 000000000..aa13106f6 --- /dev/null +++ b/tests/bug39/3.js @@ -0,0 +1,7 @@ +/*--- +flags: [qjs:track-promise-rejections] +---*/ + +const promise = Promise.reject('reject') +const error = await Promise.resolve().then(() => promise).catch(e => e) +print('Got this error:', error) From f358a1b48a6cdae575c4e724357fc735e4f0f549 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C3=BAl=20Ibarra=20Corretg=C3=A9?= Date: Thu, 8 May 2025 11:23:07 +0200 Subject: [PATCH 2/2] fixup! more resilient? --- quickjs.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/quickjs.c b/quickjs.c index a26cd5dad..f28b6cead 100644 --- a/quickjs.c +++ b/quickjs.c @@ -1260,6 +1260,8 @@ static JSValue js_promise_resolve(JSContext *ctx, JSValueConst this_val, int argc, JSValueConst *argv, int magic); static JSValue js_promise_then(JSContext *ctx, JSValueConst this_val, int argc, JSValueConst *argv); +static JSValue js_promise_resolve_thenable_job(JSContext *ctx, + int argc, JSValueConst *argv); static bool js_string_eq(JSString *p1, JSString *p2); static int js_string_compare(JSString *p1, JSString *p2); static int JS_SetPropertyValue(JSContext *ctx, JSValueConst this_obj, @@ -50223,7 +50225,7 @@ static JSValue promise_rejection_tracker_job(JSContext *ctx, int argc, JSValueConst promise; struct list_head *el, *el1; JSJobEntry *job; - bool has_other_jobs; + bool has_other_promise_jobs; assert(argc == 1); @@ -50239,15 +50241,15 @@ static JSValue promise_rejection_tracker_job(JSContext *ctx, int argc, // 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_jobs = false; + 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_rejection_tracker_job) { - has_other_jobs = true; + if (job->job_func == promise_reaction_job || job->job_func == js_promise_resolve_thenable_job) { + has_other_promise_jobs = true; break; } } - if (has_other_jobs) { + if (has_other_promise_jobs) { JS_EnqueueJob(ctx, promise_rejection_tracker_job, 1, &promise); return JS_UNDEFINED; }