Skip to content

Commit 6af7844

Browse files
Comprehensive promise handling improvements for NJS and QuickJS engines
This commit significantly enhances promise handling across multiple areas: - **Enhanced ngx_engine_njs_call() and ngx_engine_qjs_call()**: Added comprehensive promise state handling for fulfilled, rejected, and pending promises - **Fulfilled promises**: Automatically extract resolved values - **Rejected promises**: Log error - **Pending promises**: Log errors when no waiting events exist, indicating stuck promises - **Microtask queue protection**: Added iteration limit to prevent infinite microtask loops in both NJS and QuickJS engines - **Event loop starvation prevention**: Protects against recursive promise chains that could monopolize the event loop indefinitely - **Clear error reporting**: Logs specific messages when job queue limits are exceeded - **js_promise_pending.t**: Tests pending promise detection and error logging - **js_promise_rejected.t**: Validates rejected promise exception handling - **js_promise_states.t**: Comprehensive promise state transition testing - **js_infinite_loop_protection.t**: Validates microtask loop protection - **Enhanced js_async.t**: Demonstrates async function integration with js_set - **Updated stream_js.t**: Extended stream promise handling tests - Async functions now work correctly with js_set directives, and other directives where syncronious result is expected - Prevents nginx hangs from infinite microtask loops - Maintains JavaScript exception semantics for rejected promises - Provides clear error messages for debugging stuck promises - Full compatibility with both NJS and QuickJS engines This enables robust async/await patterns in nginx JavaScript while maintaining stability and providing event loop protection.
1 parent b605a4d commit 6af7844

File tree

7 files changed

+690
-9
lines changed

7 files changed

+690
-9
lines changed

nginx/ngx_js.c

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include <ngx_core.h>
1111
#include <math.h>
1212
#include "ngx_js.h"
13+
#include <njs_main.h>
1314

1415

1516
typedef struct {
@@ -683,6 +684,9 @@ ngx_njs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, void *external)
683684
}
684685

685686

687+
#define NJS_MAX_JOB_ITERATIONS 0x10000
688+
689+
686690
static ngx_int_t
687691
ngx_engine_njs_call(ngx_js_ctx_t *ctx, ngx_str_t *fname,
688692
njs_opaque_value_t *args, njs_uint_t nargs)
@@ -716,6 +720,10 @@ ngx_engine_njs_call(ngx_js_ctx_t *ctx, ngx_str_t *fname,
716720
return NGX_ERROR;
717721
}
718722

723+
/* Infinite loop protection: limit microtask queue processing. */
724+
ngx_uint_t max_job_iterations = NJS_MAX_JOB_ITERATIONS;
725+
ngx_uint_t job_count = 0;
726+
719727
for ( ;; ) {
720728
ret = njs_vm_execute_pending_job(vm);
721729
if (ret <= NJS_OK) {
@@ -729,6 +737,35 @@ ngx_engine_njs_call(ngx_js_ctx_t *ctx, ngx_str_t *fname,
729737

730738
break;
731739
}
740+
741+
job_count++;
742+
if (job_count >= max_job_iterations) {
743+
ngx_log_error(NGX_LOG_ERR, ctx->log, 0,
744+
"js job queue processing limit exceeded (%ui iterations), "
745+
"possible infinite loop in microtasks", max_job_iterations);
746+
return NGX_ERROR;
747+
}
748+
}
749+
750+
/* Check if the value is a promise and handle its state. */
751+
njs_value_t *val = njs_value_arg(&ctx->retval);
752+
if (njs_is_promise(val)) {
753+
njs_promise_t *promise = njs_promise(val);
754+
njs_promise_data_t *promise_data = njs_data(&promise->value);
755+
756+
if (promise_data->state == NJS_PROMISE_FULFILL) {
757+
*val = promise_data->result;
758+
759+
} else if (promise_data->state == NJS_PROMISE_REJECTED) {
760+
njs_vm_throw(vm, &promise_data->result);
761+
762+
} else if (promise_data->state == NJS_PROMISE_PENDING &&
763+
njs_rbtree_is_empty(&ctx->waiting_events))
764+
{
765+
ngx_log_error(NGX_LOG_ERR, ctx->log, 0,
766+
"js promise pending, no jobs, no waiting_events");
767+
return NGX_ERROR;
768+
}
732769
}
733770

734771
return njs_rbtree_is_empty(&ctx->waiting_events) ? NGX_OK : NGX_AGAIN;
@@ -754,8 +791,9 @@ ngx_engine_njs_string(ngx_engine_t *e, njs_opaque_value_t *value,
754791
{
755792
ngx_int_t rc;
756793
njs_str_t s;
794+
njs_value_t *val = njs_value_arg(value);
757795

758-
rc = ngx_js_string(e->u.njs.vm, njs_value_arg(value), &s);
796+
rc = ngx_js_string(e->u.njs.vm, val, &s);
759797

760798
str->data = s.start;
761799
str->len = s.length;
@@ -1060,6 +1098,10 @@ ngx_engine_qjs_call(ngx_js_ctx_t *ctx, ngx_str_t *fname,
10601098

10611099
rt = JS_GetRuntime(cx);
10621100

1101+
/* Infinite loop protection: limit microtask queue processing. */
1102+
ngx_uint_t max_job_iterations = NJS_MAX_JOB_ITERATIONS;
1103+
ngx_uint_t job_count = 0;
1104+
10631105
for ( ;; ) {
10641106
rc = JS_ExecutePendingJob(rt, &cx1);
10651107
if (rc <= 0) {
@@ -1074,6 +1116,38 @@ ngx_engine_qjs_call(ngx_js_ctx_t *ctx, ngx_str_t *fname,
10741116

10751117
break;
10761118
}
1119+
1120+
job_count++;
1121+
if (job_count >= max_job_iterations) {
1122+
ngx_log_error(NGX_LOG_ERR, ctx->log, 0,
1123+
"js job queue processing limit exceeded (%ui iterations), "
1124+
"possible infinite loop in microtasks", max_job_iterations);
1125+
return NGX_ERROR;
1126+
}
1127+
}
1128+
1129+
/* Check if the value is a promise and handle its state. */
1130+
JSValue retval = ngx_qjs_arg(ctx->retval);
1131+
if (JS_IsObject(retval)) {
1132+
JSPromiseStateEnum state = JS_PromiseState(cx, retval);
1133+
1134+
if (state == JS_PROMISE_FULFILLED) {
1135+
JSValue result = JS_PromiseResult(cx, retval);
1136+
JS_FreeValue(cx, ngx_qjs_arg(ctx->retval));
1137+
ngx_qjs_arg(ctx->retval) = result;
1138+
1139+
} else if (state == JS_PROMISE_REJECTED) {
1140+
JSValue rejection = JS_PromiseResult(cx, retval);
1141+
JS_FreeValue(cx, ngx_qjs_arg(ctx->retval));
1142+
ngx_qjs_arg(ctx->retval) = JS_Throw(cx, rejection);
1143+
1144+
} else if (state == JS_PROMISE_PENDING &&
1145+
njs_rbtree_is_empty(&ctx->waiting_events))
1146+
{
1147+
ngx_log_error(NGX_LOG_ERR, ctx->log, 0,
1148+
"js promise pending, no jobs, no waiting_events");
1149+
return NGX_ERROR;
1150+
}
10771151
}
10781152

10791153
return njs_rbtree_is_empty(&ctx->waiting_events) ? NGX_OK : NGX_AGAIN;
@@ -1098,7 +1172,9 @@ static ngx_int_t
10981172
ngx_engine_qjs_string(ngx_engine_t *e, njs_opaque_value_t *value,
10991173
ngx_str_t *str)
11001174
{
1101-
return ngx_qjs_dump_obj(e, ngx_qjs_arg(*value), str);
1175+
JSValue val = ngx_qjs_arg(*value);
1176+
1177+
return ngx_qjs_dump_obj(e, val, str);
11021178
}
11031179

11041180

nginx/t/js_async.t

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,10 @@ events {
3535
http {
3636
%%TEST_GLOBALS_HTTP%%
3737
38-
js_set $test_async test.set_timeout;
39-
js_set $context_var test.context_var;
40-
js_set $test_set_rv_var test.set_rv_var;
38+
js_set $test_async test.set_timeout;
39+
js_set $context_var test.context_var;
40+
js_set $test_set_rv_var test.set_rv_var;
41+
js_set $test_promise_var test.set_promise_var;
4142
4243
js_import test.js;
4344
@@ -85,6 +86,10 @@ http {
8586
return 200 $test_set_rv_var;
8687
}
8788
89+
location /promise_var {
90+
return 200 $test_promise_var;
91+
}
92+
8893
location /await_reject {
8994
js_content test.await_reject;
9095
}
@@ -198,6 +203,13 @@ $t->write_file('test.js', <<EOF);
198203
r.setReturnValue(`retval: \${a1 + a2}`);
199204
}
200205
206+
async function set_promise_var(r) {
207+
const a1 = await pr(10);
208+
const a2 = await pr(20);
209+
210+
return `retval: \${a1 + a2}`;
211+
}
212+
201213
async function timeout(ms) {
202214
return new Promise((resolve, reject) => {
203215
setTimeout(() => {
@@ -213,11 +225,11 @@ $t->write_file('test.js', <<EOF);
213225
214226
export default {njs:test_njs, set_timeout, set_timeout_data,
215227
set_timeout_many, context_var, shared_ctx, limit_rate,
216-
async_content, set_rv_var, await_reject};
228+
async_content, set_rv_var, set_promise_var, await_reject};
217229
218230
EOF
219231

220-
$t->try_run('no njs available')->plan(10);
232+
$t->try_run('no njs available')->plan(11);
221233

222234
###############################################################################
223235

@@ -229,6 +241,7 @@ like(http_get('/limit_rate'), qr/A{50}/, 'limit_rate');
229241

230242
like(http_get('/async_content'), qr/retval: AB/, 'async content');
231243
like(http_get('/set_rv_var'), qr/retval: 30/, 'set return value variable');
244+
like(http_get('/promise_var'), qr/retval: 30/, 'fulfilled promise variable');
232245

233246
http_get('/async_var');
234247
http_get('/await_reject');
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
#!/usr/bin/perl
2+
3+
# (C) Test for infinite loop protection in microtask queue processing
4+
5+
# Tests for proper handling of infinite microtask loops similar to Node.js protection.
6+
7+
###############################################################################
8+
9+
use warnings;
10+
use strict;
11+
12+
use Test::More;
13+
14+
BEGIN { use FindBin; chdir($FindBin::Bin); }
15+
16+
use lib 'lib';
17+
use Test::Nginx;
18+
19+
###############################################################################
20+
21+
select STDERR; $| = 1;
22+
select STDOUT; $| = 1;
23+
24+
my $t = Test::Nginx->new()->has(qw/http rewrite/)
25+
->write_file_expand('nginx.conf', <<'EOF');
26+
27+
%%TEST_GLOBALS%%
28+
29+
daemon off;
30+
31+
events {
32+
}
33+
34+
http {
35+
%%TEST_GLOBALS_HTTP%%
36+
37+
js_import test.js;
38+
39+
server {
40+
listen 127.0.0.1:8080;
41+
server_name localhost;
42+
43+
location /njs {
44+
js_content test.njs;
45+
}
46+
47+
location /infinite_microtasks {
48+
js_content test.infinite_microtasks;
49+
}
50+
51+
location /recursive_promises {
52+
js_content test.recursive_promises;
53+
}
54+
55+
location /normal_promise_chain {
56+
js_content test.normal_promise_chain;
57+
}
58+
59+
}
60+
}
61+
62+
EOF
63+
64+
$t->write_file('test.js', <<'EOF');
65+
function test_njs(r) {
66+
r.return(200, njs.version);
67+
}
68+
69+
function infinite_microtasks(r) {
70+
// Create an infinite microtask loop - should trigger protection limit
71+
function infiniteLoop() {
72+
return Promise.resolve().then(() => {
73+
// Recursively create more microtasks
74+
return infiniteLoop();
75+
});
76+
}
77+
78+
return infiniteLoop();
79+
}
80+
81+
function recursive_promises(r) {
82+
// Another variation of infinite microtask generation
83+
let count = 0;
84+
function createPromise() {
85+
return new Promise((resolve) => {
86+
resolve();
87+
}).then(() => {
88+
count++;
89+
// Keep creating more promises indefinitely
90+
return createPromise();
91+
});
92+
}
93+
94+
return createPromise();
95+
}
96+
97+
function normal_promise_chain(r) {
98+
// Normal promise chain that should complete without hitting the limit
99+
let result = Promise.resolve(1);
100+
101+
// Create a reasonable chain of 50 promises (well under the 1000 limit)
102+
for (let i = 0; i < 50; i++) {
103+
result = result.then((value) => value + 1);
104+
}
105+
106+
return result.then((finalValue) => {
107+
r.return(200, "completed with value: " + finalValue);
108+
});
109+
}
110+
111+
export default {njs: test_njs, infinite_microtasks, recursive_promises, normal_promise_chain};
112+
113+
EOF
114+
115+
$t->try_run('no njs available')->plan(6);
116+
117+
###############################################################################
118+
119+
# Test basic functionality
120+
like(http_get('/njs'), qr/\d+\.\d+\.\d+/, 'njs version');
121+
122+
# Test normal promise chain (should work - under the limit)
123+
like(http_get('/normal_promise_chain'), qr/completed with value: 51/, 'normal promise chain completes');
124+
125+
# Test infinite microtasks scenario - should trigger protection limit
126+
my $infinite_response = http_get('/infinite_microtasks');
127+
like($infinite_response, qr/HTTP\/1\.[01] 500|Internal Server Error/, 'infinite microtasks causes error');
128+
129+
# Test recursive promises scenario - should also trigger protection limit
130+
my $recursive_response = http_get('/recursive_promises');
131+
like($recursive_response, qr/HTTP\/1\.[01] 500|Internal Server Error/, 'recursive promises causes error');
132+
133+
$t->stop();
134+
135+
# Check error log for the specific infinite loop protection messages
136+
my $error_log = $t->read_file('error.log');
137+
138+
# Should have error messages for job queue limit exceeded
139+
ok(index($error_log, 'js job queue processing limit exceeded') > 0,
140+
'job queue limit exceeded message logged');
141+
142+
# Should mention possible infinite loop
143+
ok(index($error_log, 'possible infinite loop in microtasks') > 0,
144+
'infinite loop warning message logged');
145+
146+
###############################################################################

0 commit comments

Comments
 (0)