N-API feature request: expose API to resolve promises asynchronously without napi_create_async_work

[rolftimmermans]

My use case: I have a socket that I'm polling with libuv uv_poll_t. When an event is present I want to resolve a promise.

Calling napi_resolve_deferred() does not work for me because it does not appear to run microtasks.

Looking at the test added in this commit I need to use the following code before resolving the promise.

v8::Isolate* isolate = v8::Isolate::GetCurrent();
v8::HandleScope scope(isolate);
node::CallbackScope callback_scope(isolate, v8::Object::New(isolate), {0, 0});

// resolve promise here

This works perfectly; but now I am required to pull in both v8.h and node.h. It would be nice if there were N-API method to resolve promises with napi_resolve_deferred() asynchronously.

Note that I am not able to use napi_create_async_work()/napi_queue_async_work() for the async code in this case.

[mhdawson]

I spent some time looking at this today and have a first cut at api additions in this branch:https://github.com/mhdawson/io.js/tree/promise_resolve

It adds:

napi_open_callback_scope
napi_close_callback_scope

And this is some test code that I've been working with:

  napi_callback_scope scope = NULL;
  napi_value return_string;
  NAPI_CALL(env, napi_create_string_utf8(env,
                                         "Done",
                                         NAPI_AUTO_LENGTH,
                                         &return_string));
  napi_value resource_name;
  NAPI_CALL(env, napi_create_string_utf8(
      env, "test", NAPI_AUTO_LENGTH, &resource_name));

  napi_async_context context;
  NAPI_CALL(env, napi_async_init(env, NULL, resource_name, &context));

  napi_value resource_object;
  NAPI_CALL(env, napi_create_object(env, &resource_object));

  promise_ran = false;
  NAPI_CALL(env, napi_open_callback_scope(env,
                                          resource_object,
                                          context,
                                          &scope));

  NAPI_CALL(env, napi_resolve_deferred(env, deferred, return_string));

  NAPI_CALL(env, napi_close_callback_scope(env, scope));

What I don't quite have yet is a way to test it out properly. @rolftimmermans can you take a look and fill in a bit more info about what would make a good test to add to our addons-napi test suite.

Note that I did not incorporate the open/close of a handle scope it open/close callback so if there is not one already in force that would need to be added to the code shown above.

[mhdawson]

The other thing I'd want to poke to get a bit more detail at is why napi_create_async_work()/napi_queue_async_work() cannot be used.

[rolftimmermans]

I'll have a look soon to see how this fits in my code.

The reason napi_create_async_work()/napi_queue_async_work() is not adequate is that the promise will be resolved by polling on a socket (with libuv and uv_poll_t). So it's not a pattern that fits into the idea of doing a fixed amount of work and then resolve a promise afterwards.

[mhdawson]

Just wondering if scheduling aysnc work, doing the fixed amount of work and then scheduling another async work for the next time has issues ?

[mhdawson]

Possibly related, also with suggested API addition #13512

[mhdawson]

Sorry looks like I had the wrong branch in the post above. Looking for the right one now.

[mhdawson]

I think I had the write branch may just not have add changes. Will recreate.

[mhdawson]

So new branch is here: https://github.com/mhdawson/io.js/tree/callback_scope (top commit). Working towards a PR as @addaleax suggested it would be good to expose this. @rolftimmermans would still like your input as to whether is addresses your use case.

It has tests which are the ported versions from the non-N-API addon tests, but test-async-hooks.js is failing for some reason. The correct async id is not passed to the registered hook. From some initial investigation it seems to be ok when the node::CallbackScope object is created inside of napi_open_callback_scope , but is not correct when emitBeforeScript is called.

I'm still investigating but @addaleax if you have any suggestions off the top of your head from the symptoms please let me know.

[rolftimmermans]

This seems to be working fine, thanks! With this change I am able to build the project without including v8.h (I think I still need node.h for node::AtExit, but that's another matter).

The test suite passes. I'll try to see if I manage to break it if I try hard enough, but for now it's looking good.

The C-API wasn't very ergonomic to use, but I imagine that can be solved in the node-addon-api project.

[rolftimmermans]

Btw, I've been passing in NULL for the second argument to napi_open_callback_scope. Not sure if it is explicitly supported but it seems to work.

[mhdawson]

@rolftimmermans Thanks for he confirmation. I think passing in may be ok, I still need to add the documentation and I'll make sure to check and capture there. Like;y will get to that early in Jan as I still need to figure out the failure in the async-hooks test and I'm off on holiday starting Friday.

[Globik]

@mhdawson why https://github.com/Globik/libqrencode-js/blob/master/simple_test/test_async.c
does not work properly? I mean when calling the same async function from nodejs part twice and more times brings the process to crashing? Race condition? Or should it be wrapping into the handle scoping? But with no luck also(

[nathansobo]

👍 It would be great to see a more flexible way of entering a callback scope get added to the stable API.

[mhdawson]

This came up in the TSE meeting today and I'm going to rebase and then open a Work in progress PR mentioning the test that I still don't have working. @addaleax will then help me figure out what the issue is.

[mhdawson]

PR here #18089

[mhdawson]

@rolftimmermans, finally got back to this, resolved issues in tests and landed it.

[rolftimmermans]

I must be doing something incorrectly, but I'm hitting a v8 assertion when I attempt to use this feature in Node 9.6 or 9.7 or master.

I am calling napi_open_callback_scope as described above (and I swear it used to work when testing against the PR, though I can't double check easily right now). The scope is opened from a completion callback of uv_work.

My program aborts with the following trace:

FATAL ERROR: v8::HandleScope::CreateHandle() Cannot create a handle without a HandleScope
 1: node::Abort() [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
 2: node::OnFatalError(char const*, char const*) [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
 3: v8::Utils::ReportApiFailure(char const*, char const*) [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
 4: v8::internal::HandleScope::Extend(v8::internal::Isolate*) [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
 5: v8::Isolate::GetCurrentContext() [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
 6: napi_open_callback_scope [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
 7: zmq::CallbackScope::CallbackScope(napi_env__*, napi_async_context__*, napi_value__*) [/Users/rolftimmermans/Code/vendor/zmq2/lib/binary/zeromq-ng.node]
 8: std::__1::__function::__func<zmq::Socket::Bind(Napi::CallbackInfo const&)::'lambda0'(), std::__1::allocator<zmq::Socket::Bind(Napi::CallbackInfo const&)::'lambda0'()>, void ()>::operator()() [/Users/rolftimmermans/Code/vendor/zmq2/lib/binary/zeromq-ng.node]
 9: zmq::Work::Exec()::'lambda'(uv_work_s*, int)::__invoke(uv_work_s*, int) [/Users/rolftimmermans/Code/vendor/zmq2/lib/binary/zeromq-ng.node]
10: uv__work_done [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
11: uv__async_io [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
12: uv__io_poll [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
13: uv_run [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
14: node::Start(v8::Isolate*, node::IsolateData*, int, char const* const*, int, char const* const*) [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
15: node::Start(uv_loop_s*, int, char const* const*, int, char const* const*) [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
16: node::Start(int, char**) [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
17: start [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
18: 0xc

It seems I'm running into an assertion in V8 on these lines:

node/deps/v8/src/handles.cc

Lines 72 to 76 in 4c4af64

	if (!Utils::ApiCheck(current->level != current->sealed_level,
	"v8::HandleScope::CreateHandle()",
	"Cannot create a handle without a HandleScope")) {
	return nullptr;
	}

Unfortunately my knowledge of this change and V8 is too limited to understand why I'm running into this.

@mhdawson Perhaps you could give me any pointers to troubleshoot?

[TimothyGu]

Declaring a v8::HandleScope before the GetCurrentContext call in napi_open_callback_scope should be able to fix this.

[rolftimmermans]

Declaring a v8::HandleScope before the GetCurrentContext call in napi_open_callback_scope should be able to fix this.

It works, sort of – except the crash is now moved to the API call that attempts to create a V8 object (in this case napi_resolve_deferred).

It seems napi_open_callback_scope needs to keep a v8::HandleScope around somehow? That is, not just for the duration of napi_open_callback_scope itself but until napi_close_callback_scope is called. How would that have to be done?

Crash is now:

FATAL ERROR: v8::HandleScope::CreateHandle() Cannot create a handle without a HandleScope
 1: node::Abort() [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
 2: node::OnFatalError(char const*, char const*) [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
 3: v8::Utils::ReportApiFailure(char const*, char const*) [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
 4: v8::internal::HandleScope::Extend(v8::internal::Isolate*) [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
 5: v8::Isolate::GetCurrentContext() [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
 6: (anonymous namespace)::v8impl::ConcludeDeferred(napi_env__*, napi_deferred__*, napi_value__*, bool) [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
 7: std::__1::__function::__func<zmq::Socket::Bind(Napi::CallbackInfo const&)::'lambda0'(), std::__1::allocator<zmq::Socket::Bind(Napi::CallbackInfo const&)::'lambda0'()>, void ()>::operator()() [/Users/rolftimmermans/Code/vendor/zmq2/lib/binary/zeromq-ng.node]
 8: zmq::Work::Exec()::'lambda'(uv_work_s*, int)::__invoke(uv_work_s*, int) [/Users/rolftimmermans/Code/vendor/zmq2/lib/binary/zeromq-ng.node]
 9: uv__work_done [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
10: uv__async_io [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
11: uv__io_poll [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
12: uv_run [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
13: node::Start(v8::Isolate*, node::IsolateData*, int, char const* const*, int, char const* const*) [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
14: node::Start(uv_loop_s*, int, char const* const*, int, char const* const*) [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
15: node::Start(int, char**) [/Users/rolftimmermans/Code/vendor/node/out/Release/node]
16: start [/Users/rolftimmermans/Code/vendor/node/out/Release/node]

[addaleax]

You always need to have a napi_open_handle_scope call before napi_open_callback_scope, and the reverse order for the close calls.

Without a handle scope, how would you pass resource_object to napi_open_callback_scope to begin with?

[rolftimmermans]

You always need to have a napi_open_handle_scope call before napi_open_callback_scope, and the reverse order for the close calls.

Right, OK! Somehow I missed that. I wonder if I misremember how I tested it previously or if something is different between the original PR and how it landed.

Anyway. Without any changes to Node whatsoever the code works if I call napi_open_handle_scope/napi_open_callback_scope in the appropriate order.

The precise order of API calls that are required was definitely not straightforward for me, but to make this easier I guess that's why we have nodejs/node-addon-api#223.

Thanks! Feel free to ignore this report otherwise.

Without a handle scope, how would you pass resource_object to napi_open_callback_scope to begin with?

I seem to remember I had been passing nullptr in the past; that definitely doesn't work anymore.