Skip to content

Commit

Permalink
Fixed resolving when Promise is inherited.
Browse files Browse the repository at this point in the history
Previously, njs_promise_resolve() might return njs_object_t instead of
njs_promise_t. Later an instance of njs_object_t was put into a
NJS_PROMISE value. Whereas njs_promise_t is always expected to be inside
of a NJS_PROMISE value.

This closes #813 issue on Github.
  • Loading branch information
xeioex committed Nov 8, 2024
1 parent 72f0b5d commit 5247aac
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 29 deletions.
37 changes: 14 additions & 23 deletions src/njs_promise.c
Original file line number Diff line number Diff line change
Expand Up @@ -675,27 +675,19 @@ static njs_int_t
njs_promise_object_resolve(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
njs_index_t unused, njs_value_t *retval)
{
njs_promise_t *promise;

if (njs_slow_path(!njs_is_object(njs_argument(args, 0)))) {
njs_type_error(vm, "this value is not an object");
return NJS_ERROR;
}

promise = njs_promise_resolve(vm, njs_argument(args, 0),
njs_arg(args, nargs, 1));
if (njs_slow_path(promise == NULL)) {
return NJS_ERROR;
}

njs_set_promise(retval, promise);

return NJS_OK;
return njs_promise_resolve(vm, njs_argument(args, 0),
njs_arg(args, nargs, 1), retval);
}


njs_promise_t *
njs_promise_resolve(njs_vm_t *vm, njs_value_t *constructor, njs_value_t *x)
njs_int_t
njs_promise_resolve(njs_vm_t *vm, njs_value_t *constructor, njs_value_t *x,
njs_value_t *retval)
{
njs_int_t ret;
njs_value_t value;
Expand All @@ -707,26 +699,28 @@ njs_promise_resolve(njs_vm_t *vm, njs_value_t *constructor, njs_value_t *x)
ret = njs_value_property(vm, x, njs_value_arg(&string_constructor),
&value);
if (njs_slow_path(ret == NJS_ERROR)) {
return NULL;
return NJS_ERROR;
}

if (njs_values_same(&value, constructor)) {
return njs_promise(x);
njs_value_assign(retval, x);
return NJS_OK;
}
}

capability = njs_promise_new_capability(vm, constructor);
if (njs_slow_path(capability == NULL)) {
return NULL;
return NJS_ERROR;
}

ret = njs_function_call(vm, njs_function(&capability->resolve),
&njs_value_undefined, x, 1, &value);
if (njs_slow_path(ret != NJS_OK)) {
return NULL;
return ret;
}

return njs_promise(&capability->promise);
njs_value_assign(retval, &capability->promise);
return NJS_OK;
}


Expand Down Expand Up @@ -1017,7 +1011,6 @@ njs_promise_then_finally_function(njs_vm_t *vm, njs_value_t *args,
{
njs_int_t ret;
njs_value_t value, argument;
njs_promise_t *promise;
njs_function_t *function;
njs_native_frame_t *frame;
njs_promise_context_t *context;
Expand All @@ -1031,13 +1024,11 @@ njs_promise_then_finally_function(njs_vm_t *vm, njs_value_t *args,
return ret;
}

promise = njs_promise_resolve(vm, &context->constructor, &value);
if (njs_slow_path(promise == NULL)) {
ret = njs_promise_resolve(vm, &context->constructor, &value, &value);
if (njs_slow_path(ret != NJS_OK)) {
return NJS_ERROR;
}

njs_set_promise(&value, promise);

function = njs_promise_create_function(vm, sizeof(njs_value_t));
if (njs_slow_path(function == NULL)) {
return NJS_ERROR;
Expand Down
4 changes: 2 additions & 2 deletions src/njs_promise.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ njs_function_t *njs_promise_create_function(njs_vm_t *vm, size_t context_size);
njs_int_t njs_promise_perform_then(njs_vm_t *vm, njs_value_t *value,
njs_value_t *fulfilled, njs_value_t *rejected,
njs_promise_capability_t *capability, njs_value_t *retval);
njs_promise_t *njs_promise_resolve(njs_vm_t *vm, njs_value_t *constructor,
njs_value_t *x);
njs_int_t njs_promise_resolve(njs_vm_t *vm, njs_value_t *constructor,
njs_value_t *x, njs_value_t *retval);


extern const njs_object_type_init_t njs_promise_type_init;
Expand Down
6 changes: 2 additions & 4 deletions src/njs_vmcode.c
Original file line number Diff line number Diff line change
Expand Up @@ -2637,7 +2637,6 @@ njs_vmcode_await(njs_vm_t *vm, njs_vmcode_await_t *await,
njs_int_t ret;
njs_frame_t *frame;
njs_value_t ctor, val, on_fulfilled, on_rejected, *value, retval;
njs_promise_t *promise;
njs_function_t *fulfilled, *rejected;
njs_native_frame_t *active;

Expand All @@ -2651,8 +2650,8 @@ njs_vmcode_await(njs_vm_t *vm, njs_vmcode_await_t *await,

njs_set_function(&ctor, &njs_vm_ctor(vm, NJS_OBJ_TYPE_PROMISE));

promise = njs_promise_resolve(vm, &ctor, value);
if (njs_slow_path(promise == NULL)) {
ret = njs_promise_resolve(vm, &ctor, value, &val);
if (njs_slow_path(ret != NJS_OK)) {
return NJS_ERROR;
}

Expand Down Expand Up @@ -2710,7 +2709,6 @@ njs_vmcode_await(njs_vm_t *vm, njs_vmcode_await_t *await,
rejected->args_count = 1;
rejected->u.native = njs_await_rejected;

njs_set_promise(&val, promise);
njs_set_function(&on_fulfilled, fulfilled);
njs_set_function(&on_rejected, rejected);

Expand Down
34 changes: 34 additions & 0 deletions test/js/promise_s27.t.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*---
includes: []
flags: [async]
---*/

var inherits = (child, parent) => {
child.prototype = Object.create(parent.prototype, {
constructor: {
value: child,
enumerable: false,
writable: true,
configurable: true
}
});
Object.setPrototypeOf(child, parent);
};

function BoxedPromise(executor) {
var context, args;
new Promise(wrappedExecutor);
executor.apply(context, args);

function wrappedExecutor(resolve, reject) {
context = this;
args = [v => resolve(v),v => reject(v)];
}
}

inherits(BoxedPromise, Promise);

Promise.resolve()
.then(() => BoxedPromise.resolve())
.catch(e => assert.sameValue(e.constructor, TypeError))
.then($DONE, $DONE);

0 comments on commit 5247aac

Please sign in to comment.