Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

N-API: pass error from execute callback in napi_create_async_work to main loop #20964

Closed
mcheshkov opened this issue May 25, 2018 · 3 comments
Closed

N-API: pass error from execute callback in napi_create_async_work to main loop #20964

mcheshkov opened this issue May 25, 2018 · 3 comments

Comments

@mcheshkov
Copy link

  • Version: v8.4.0
  • Platform: Linux 4.13.0-43-generic Improve repo description #48~16.04.1-Ubuntu SMP Thu May 17 12:56:46 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
  • Subsystem: N-API

I can't find a way to pass error from simple async operation back to main loop.
Tests for async calls napi_throw_type_error straight from execute callback, which should run in thread pool. But napi_throw_type_error does not synchronize access to VM, which, AFAIK, is not allowed, by v8 at least.

node/test/addons-napi/test_async/test_async.cc

Lines 23 to 37 in a406a32

void Execute(napi_env env, void* data) {
#if defined _WIN32
Sleep(1000);
#else
sleep(1);
#endif
carrier* c = static_cast<carrier*>(data);
if (c != &the_carrier) {
napi_throw_type_error(env, nullptr, "Wrong data parameter to Execute.");
return;
}
c->_output = c->_input * 2;
}

What is correct way to pass error back? I can make my own variant using data pointer, but something like Nan::AsyncWorker::SetErrorMessage would be nice.
Availability of napi_env in napi_async_execute_callback make things more confusing - it makes me think that I can call any N-API function.
@mhdawson
Copy link
Member

I'd need more context to understand which, but I'm guessing you want either

@mcheshkov
Copy link
Author

#17887 is great, subscribing.

I have two concerns. Both are about "Simple Asynchronous Operations" as in documentation, running single function in threadpool, parallel to JS thread.

With NAN one could write something like that

class Worker : public Nan::AsyncWorker {
 public:
  void Execute () {
    if (true /*error happened*/) { SetErrorMessage("Something bad happened"); }
  }
};

SetErrorMessage will save message in worker instance field, then libuv will call in JS thread AsyncExecuteComplete -> WorkComplete and it will call either HandleOKCallback or HandleErrorCallback, depending on saved message.

With N-API you have to do this by hand - define structure for worker context with error message, pass it as data pointer, save message there, handle it in complete callback. It is more flexible, but, still, extra work for (as I see it) default case of running async operation.

napi_async_execute_callback executes in thread from thread pool.
One mustn't access VM from other threads, at least in v8.
But napi_async_execute_callback receives napi_env as argument, and make me think I can use it to create new JS values, call functions or throw exceptions.

gabrielschulhof pushed a commit that referenced this issue Jun 29, 2018
n-api: add API for asynchronous functions
81f06ba 
Bundle a `uv_async_t`, a `uv_idle_t`, a `uv_mutex_t`, a `uv_cond_t`,
and a `v8::Persistent<v8::Function>` to make it possible to call into JS
from another thread. The API accepts a void data pointer and a callback
which will be invoked on the loop thread and which will receive the
`napi_value` representing the JavaScript function to call so as to
perform the call into JS. The callback is run inside a
`node::CallbackScope`.

A `std::queue<void*>` is used to store calls from the secondary
threads, and an idle loop is started by the `uv_async_t` callback on the
loop thread to drain the queue, calling into JS with each item.

Items can be added to the queue blockingly or non-blockingly.

The thread-safe function can be referenced or unreferenced, with the
same semantics as libuv handles.

Re: nodejs/help#1035
Re: #20964
Fixes: #13512
PR-URL: #17887
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
targos pushed a commit that referenced this issue Jun 30, 2018
@targos
n-api: add API for asynchronous functions
91384bf 
Bundle a `uv_async_t`, a `uv_idle_t`, a `uv_mutex_t`, a `uv_cond_t`,
and a `v8::Persistent<v8::Function>` to make it possible to call into JS
from another thread. The API accepts a void data pointer and a callback
which will be invoked on the loop thread and which will receive the
`napi_value` representing the JavaScript function to call so as to
perform the call into JS. The callback is run inside a
`node::CallbackScope`.

A `std::queue<void*>` is used to store calls from the secondary
threads, and an idle loop is started by the `uv_async_t` callback on the
loop thread to drain the queue, calling into JS with each item.

Items can be added to the queue blockingly or non-blockingly.

The thread-safe function can be referenced or unreferenced, with the
same semantics as libuv handles.

Re: nodejs/help#1035
Re: #20964
Fixes: #13512
PR-URL: #17887
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this issue Dec 28, 2018
n-api: add API for asynchronous functions
2e15372 
Bundle a `uv_async_t`, a `uv_idle_t`, a `uv_mutex_t`, a `uv_cond_t`,
and a `v8::Persistent<v8::Function>` to make it possible to call into JS
from another thread. The API accepts a void data pointer and a callback
which will be invoked on the loop thread and which will receive the
`napi_value` representing the JavaScript function to call so as to
perform the call into JS. The callback is run inside a
`node::CallbackScope`.

A `std::queue<void*>` is used to store calls from the secondary
threads, and an idle loop is started by the `uv_async_t` callback on the
loop thread to drain the queue, calling into JS with each item.

Items can be added to the queue blockingly or non-blockingly.

The thread-safe function can be referenced or unreferenced, with the
same semantics as libuv handles.

Re: nodejs/help#1035
Re: nodejs#20964
Fixes: nodejs#13512
PR-URL: nodejs#17887
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this issue Jan 18, 2019
@MylesBorins
n-api: add API for asynchronous functions
64f232c 
Bundle a `uv_async_t`, a `uv_idle_t`, a `uv_mutex_t`, a `uv_cond_t`,
and a `v8::Persistent<v8::Function>` to make it possible to call into JS
from another thread. The API accepts a void data pointer and a callback
which will be invoked on the loop thread and which will receive the
`napi_value` representing the JavaScript function to call so as to
perform the call into JS. The callback is run inside a
`node::CallbackScope`.

A `std::queue<void*>` is used to store calls from the secondary
threads, and an idle loop is started by the `uv_async_t` callback on the
loop thread to drain the queue, calling into JS with each item.

Items can be added to the queue blockingly or non-blockingly.

The thread-safe function can be referenced or unreferenced, with the
same semantics as libuv handles.

Re: nodejs/help#1035
Re: #20964
Fixes: #13512
Backport-PR-URL: #25002
PR-URL: #17887
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
rvagg pushed a commit that referenced this issue Feb 28, 2019
@rvagg
n-api: add API for asynchronous functions
759a018 
Bundle a `uv_async_t`, a `uv_idle_t`, a `uv_mutex_t`, a `uv_cond_t`,
and a `v8::Persistent<v8::Function>` to make it possible to call into JS
from another thread. The API accepts a void data pointer and a callback
which will be invoked on the loop thread and which will receive the
`napi_value` representing the JavaScript function to call so as to
perform the call into JS. The callback is run inside a
`node::CallbackScope`.

A `std::queue<void*>` is used to store calls from the secondary
threads, and an idle loop is started by the `uv_async_t` callback on the
loop thread to drain the queue, calling into JS with each item.

Items can be added to the queue blockingly or non-blockingly.

The thread-safe function can be referenced or unreferenced, with the
same semantics as libuv handles.

Re: nodejs/help#1035
Re: #20964
Fixes: #13512
Backport-PR-URL: #25002
PR-URL: #17887
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@snyk-bot snyk-bot mentioned this issue Apr 1, 2020
Open
@jasnell
Copy link
Member

jasnell commented Jun 19, 2020

I believe this has been resolved. Closing. Can reopen if necessary.

@jasnell jasnell closed this as completed Jun 19, 2020
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this issue May 5, 2024
n-api: add API for asynchronous functions
6a4f191 
Bundle a `uv_async_t`, a `uv_idle_t`, a `uv_mutex_t`, a `uv_cond_t`,
and a `v8::Persistent<v8::Function>` to make it possible to call into JS
from another thread. The API accepts a void data pointer and a callback
which will be invoked on the loop thread and which will receive the
`napi_value` representing the JavaScript function to call so as to
perform the call into JS. The callback is run inside a
`node::CallbackScope`.

A `std::queue<void*>` is used to store calls from the secondary
threads, and an idle loop is started by the `uv_async_t` callback on the
loop thread to drain the queue, calling into JS with each item.

Items can be added to the queue blockingly or non-blockingly.

The thread-safe function can be referenced or unreferenced, with the
same semantics as libuv handles.

Re: nodejs/help#1035
Re: nodejs/node#20964
Fixes: nodejs/node#13512
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jasnell @mcheshkov @mhdawson