Mercurial > njs
changeset 2574:eead47ce1ad2
Fixed frame saving for awaited function with closures.
The issue was introduced in 6335367 (0.7.0).
This fixes #530 issue on Github.
| author | Dmitry Volyntsev <xeioex@nginx.com> |
|---|---|
| date | Wed, 11 Jun 2025 16:17:42 -0700 |
| parents | d925481ac836 |
| children | 9be6834ec6ff |
| files | src/njs_function.c src/njs_function.h test/js/async_closure.t.js test/js/async_closure_arg.t.js test/js/async_closure_share.t.js |
| diffstat | 5 files changed, 133 insertions(+), 63 deletions(-) [+] |
line wrap: on
line diff
--- a/src/njs_function.c Mon Jun 09 18:38:15 2025 -0700 +++ b/src/njs_function.c Wed Jun 11 16:17:42 2025 -0700 @@ -394,6 +394,19 @@ 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_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 @@ 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); + /* + * 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. + */ + + map = (njs_value_t **) ((u_char *) native + NJS_FRAME_SIZE); + value = (njs_value_t *) (map + value_count); + + current_map = (njs_value_t **) ((u_char *) active + NJS_FRAME_SIZE); + + 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]); + + } else { + map[n] = current_map[n]; + } + } native->arguments = value; - native->local = new + njs_function_frame_args_count(active); + native->local = map + args_count; native->pc = pc; - start = njs_function_frame_values(active, &end); - p = native->arguments; - - while (start < end) { - njs_value_assign(p, start++); - *new++ = p++; - } - - /* Move all arguments. */ - - p = native->arguments; - local = native->local + 1 /* this */; - - for (n = 0; n < function->args_count; n++) { - if (!njs_is_valid(p)) { - njs_set_undefined(p); - } - - *local++ = p++; - } - return NJS_OK; } @@ -746,7 +762,6 @@ 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 @@ frame = frame->previous; } - start = frame; - end = frame->free; - closure = njs_function_closures(function); n = lambda->nclosures; @@ -772,7 +784,7 @@ 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_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_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 @@ native = native->previous; } - start = native; - end = native->free; - indexes = lambda->closures; refs = njs_function_closures(function); @@ -851,12 +859,12 @@ 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:
--- a/src/njs_function.h Mon Jun 09 18:38:15 2025 -0700 +++ b/src/njs_function.h Wed Jun 11 16:17:42 2025 -0700 @@ -202,29 +202,15 @@ } -njs_inline size_t -njs_function_frame_args_count(njs_native_frame_t *frame) +njs_inline njs_bool_t +njs_is_value_allocated_on_frame(njs_native_frame_t *frame, njs_value_t *value) { - uintptr_t start; - - start = (uintptr_t) ((u_char *) frame + NJS_FRAME_SIZE); - - return ((uintptr_t) frame->local - start) / sizeof(njs_value_t *); -} - + void *start, *end; -njs_inline njs_value_t * -njs_function_frame_values(njs_native_frame_t *frame, njs_value_t **end) -{ - size_t count; - uintptr_t start; + start = frame; + end = frame->free; - start = (uintptr_t) ((u_char *) frame + NJS_FRAME_SIZE); - count = ((uintptr_t) frame->arguments - start) / sizeof(njs_value_t *); - - *end = frame->arguments + count; - - return frame->arguments; + return start <= (void *) value && (void *) value < end; }
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/js/async_closure.t.js Wed Jun 11 16:17:42 2025 -0700 @@ -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);
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/js/async_closure_arg.t.js Wed Jun 11 16:17:42 2025 -0700 @@ -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);
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/js/async_closure_share.t.js Wed Jun 11 16:17:42 2025 -0700 @@ -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);
