details:
https://github.com/nginx/njs/commit/ef7ef9fe94fdcb029f5a61bed64067e303d4bb49
branches: master
commit: ef7ef9fe94fdcb029f5a61bed64067e303d4bb49
user: Dmitry Volyntsev <xei...@nginx.com>
date: Wed, 11 Jun 2025 16:17:42 -0700
description:
Fixed frame saving for awaited function with closures.
The issue was introduced in 6335367 (0.7.0).
This fixes #530 issue on Github.
---
src/njs_function.c | 88 ++++++++++++++++++++++------------------
src/njs_function.h | 26 +++---------
test/js/async_closure.t.js | 24 +++++++++++
test/js/async_closure_arg.t.js | 24 +++++++++++
test/js/async_closure_share.t.js | 28 +++++++++++++
5 files changed, 130 insertions(+), 60 deletions(-)
diff --git a/src/njs_function.c b/src/njs_function.c
index 06948177..90a54204 100644
--- a/src/njs_function.c
+++ b/src/njs_function.c
@@ -394,6 +394,19 @@ njs_function_lambda_frame(njs_vm_t *vm, njs_function_t
*function,
lambda = function->u.lambda;
+ /*
+ * Lambda frame has the following layout:
+ * njs_frame_t | p0 , p2, ..., pn | v0, v1, ..., vn
+ * where:
+ * p0, p1, ..., pn - pointers to arguments and locals,
+ * v0, v1, ..., vn - values of arguments and locals.
+ * n - number of arguments + locals.
+ *
+ * Normally, the pointers point to the values directly after them,
+ * but if a value was captured as a closure by an inner function,
+ * pn points to a value allocated from the heap.
+ */
+
args_count = njs_max(nargs, lambda->nargs);
value_count = args_count + lambda->nlocal;
@@ -675,11 +688,11 @@ njs_function_frame_free(njs_vm_t *vm, njs_native_frame_t
*native)
njs_int_t
njs_function_frame_save(njs_vm_t *vm, njs_frame_t *frame, u_char *pc)
{
- size_t args_count, value_count, n;
- njs_value_t *start, *end, *p, **new, *value, **local;
- njs_function_t *function;
+ size_t args_count, value_count, n;
+ njs_value_t **map, *value, **current_map;
+ njs_function_t *function;
+ njs_native_frame_t *active, *native;
njs_function_lambda_t *lambda;
- njs_native_frame_t *active, *native;
*frame = *vm->active_frame;
@@ -697,34 +710,37 @@ njs_function_frame_save(njs_vm_t *vm, njs_frame_t *frame,
u_char *pc)
args_count = njs_max(native->nargs, lambda->nargs);
value_count = args_count + lambda->nlocal;
- new = (njs_value_t **) ((u_char *) native + NJS_FRAME_SIZE);
- value = (njs_value_t *) (new + value_count);
-
- native->arguments = value;
- native->local = new + njs_function_frame_args_count(active);
- native->pc = pc;
-
- start = njs_function_frame_values(active, &end);
- p = native->arguments;
+ /*
+ * We need to save the current frame state because it will be freed
+ * when the function returns.
+ *
+ * To detect whether a value is captured as a closure,
+ * we check whether the pointer is within the frame. In this case
+ * the pointer is copied as is because the value it points to
+ * is already allocated in the heap and will not be freed.
+ * See njs_function_capture_closure() and njs_function_lambda_frame()
+ * for details.
+ */
- while (start < end) {
- njs_value_assign(p, start++);
- *new++ = p++;
- }
+ map = (njs_value_t **) ((u_char *) native + NJS_FRAME_SIZE);
+ value = (njs_value_t *) (map + value_count);
- /* Move all arguments. */
+ current_map = (njs_value_t **) ((u_char *) active + NJS_FRAME_SIZE);
- p = native->arguments;
- local = native->local + 1 /* this */;
+ for (n = 0; n < value_count; n++) {
+ if (njs_is_value_allocated_on_frame(active, current_map[n])) {
+ map[n] = &value[n];
+ njs_value_assign(&value[n], current_map[n]);
- for (n = 0; n < function->args_count; n++) {
- if (!njs_is_valid(p)) {
- njs_set_undefined(p);
+ } else {
+ map[n] = current_map[n];
}
-
- *local++ = p++;
}
+ native->arguments = value;
+ native->local = map + args_count;
+ native->pc = pc;
+
return NJS_OK;
}
@@ -746,7 +762,6 @@ njs_int_t
njs_function_capture_closure(njs_vm_t *vm, njs_function_t *function,
njs_function_lambda_t *lambda)
{
- void *start, *end;
uint32_t n;
njs_value_t *value, **closure;
njs_native_frame_t *frame;
@@ -761,9 +776,6 @@ njs_function_capture_closure(njs_vm_t *vm, njs_function_t
*function,
frame = frame->previous;
}
- start = frame;
- end = frame->free;
-
closure = njs_function_closures(function);
n = lambda->nclosures;
@@ -772,7 +784,7 @@ njs_function_capture_closure(njs_vm_t *vm, njs_function_t
*function,
value = njs_scope_value(vm, lambda->closures[n]);
- if (start <= (void *) value && (void *) value < end) {
+ if (njs_is_value_allocated_on_frame(frame, value)) {
value = njs_scope_value_clone(vm, lambda->closures[n], value);
if (njs_slow_path(value == NULL)) {
return NJS_ERROR;
@@ -788,14 +800,14 @@ njs_function_capture_closure(njs_vm_t *vm, njs_function_t
*function,
njs_inline njs_value_t *
-njs_function_closure_value(njs_vm_t *vm, njs_value_t **scope, njs_index_t
index,
- void *start, void *end)
+njs_function_closure_value(njs_vm_t *vm, njs_native_frame_t *frame,
+ njs_value_t **scope, njs_index_t index)
{
njs_value_t *value, *newval;
value = scope[njs_scope_index_value(index)];
- if (start <= (void *) value && end > (void *) value) {
+ if (njs_is_value_allocated_on_frame(frame, value)) {
newval = njs_mp_alloc(vm->mem_pool, sizeof(njs_value_t));
if (njs_slow_path(newval == NULL)) {
njs_memory_error(vm);
@@ -815,7 +827,6 @@ njs_function_closure_value(njs_vm_t *vm, njs_value_t
**scope, njs_index_t index,
njs_int_t
njs_function_capture_global_closures(njs_vm_t *vm, njs_function_t *function)
{
- void *start, *end;
uint32_t n;
njs_value_t *value, **refs, **global;
njs_index_t *indexes, index;
@@ -834,9 +845,6 @@ njs_function_capture_global_closures(njs_vm_t *vm,
njs_function_t *function)
native = native->previous;
}
- start = native;
- end = native->free;
-
indexes = lambda->closures;
refs = njs_function_closures(function);
@@ -851,12 +859,12 @@ njs_function_capture_global_closures(njs_vm_t *vm,
njs_function_t *function)
switch (njs_scope_index_type(index)) {
case NJS_LEVEL_LOCAL:
- value = njs_function_closure_value(vm, native->local, index,
- start, end);
+ value = njs_function_closure_value(vm, native, native->local,
+ index);
break;
case NJS_LEVEL_GLOBAL:
- value = njs_function_closure_value(vm, global, index, start, end);
+ value = njs_function_closure_value(vm, native, global, index);
break;
default:
diff --git a/src/njs_function.h b/src/njs_function.h
index 5aa98dd3..1b5de544 100644
--- a/src/njs_function.h
+++ b/src/njs_function.h
@@ -202,29 +202,15 @@ njs_function_frame_size(njs_native_frame_t *frame)
}
-njs_inline size_t
-njs_function_frame_args_count(njs_native_frame_t *frame)
-{
- uintptr_t start;
-
- start = (uintptr_t) ((u_char *) frame + NJS_FRAME_SIZE);
-
- return ((uintptr_t) frame->local - start) / sizeof(njs_value_t *);
-}
-
-
-njs_inline njs_value_t *
-njs_function_frame_values(njs_native_frame_t *frame, njs_value_t **end)
+njs_inline njs_bool_t
+njs_is_value_allocated_on_frame(njs_native_frame_t *frame, njs_value_t *value)
{
- size_t count;
- uintptr_t start;
-
- start = (uintptr_t) ((u_char *) frame + NJS_FRAME_SIZE);
- count = ((uintptr_t) frame->arguments - start) / sizeof(njs_value_t *);
+ void *start, *end;
- *end = frame->arguments + count;
+ start = frame;
+ end = frame->free;
- return frame->arguments;
+ return start <= (void *) value && (void *) value < end;
}
diff --git a/test/js/async_closure.t.js b/test/js/async_closure.t.js
new file mode 100644
index 00000000..9a387ff2
--- /dev/null
+++ b/test/js/async_closure.t.js
@@ -0,0 +1,24 @@
+/*---
+includes: []
+flags: [async]
+---*/
+
+async function f() {
+ await 1;
+ var v = 2;
+
+ function g() {
+ return v + 1;
+ }
+
+ function s() {
+ g + 1;
+ }
+
+ return g();
+}
+
+f().then(v => {
+ assert.sameValue(v, 3)
+})
+.then($DONE, $DONE);
diff --git a/test/js/async_closure_arg.t.js b/test/js/async_closure_arg.t.js
new file mode 100644
index 00000000..d6aa8ab6
--- /dev/null
+++ b/test/js/async_closure_arg.t.js
@@ -0,0 +1,24 @@
+/*---
+includes: []
+flags: [async]
+---*/
+
+async function f(v) {
+ await 1;
+ v = 2;
+
+ function g() {
+ return v + 1;
+ }
+
+ function s() {
+ g + 1;
+ }
+
+ return g();
+}
+
+f(42).then(v => {
+ assert.sameValue(v, 3)
+})
+.then($DONE, $DONE);
diff --git a/test/js/async_closure_share.t.js b/test/js/async_closure_share.t.js
new file mode 100644
index 00000000..d78f92c5
--- /dev/null
+++ b/test/js/async_closure_share.t.js
@@ -0,0 +1,28 @@
+/*---
+includes: []
+flags: [async]
+---*/
+
+async function f() {
+ await 1;
+ var v = 'f';
+
+ function g() {
+ v += ':g';
+ return v;
+ }
+
+ function s() {
+ v += ':s';
+ return v;
+ }
+
+ return [g, s];
+}
+
+f().then(pair => {
+ pair[0]();
+ var v = pair[1]();
+ assert.sameValue(v, 'f:g:s');
+})
+.then($DONE, $DONE);
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
