| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Sorry, I have no idea why npm run lint not work on my local machine. Manually do Alt + Shift + F use the clang-format in vscode, request run CI again. |
Sorry, something went wrong.
There was a problem hiding this comment.
Hey Toyo, thanks for the work!
I'm wondering if classes like AsyncWorker should be scoped with NAPI_HAS_THREADS too?
Sorry, something went wrong.
|
Because emnapi napi_*_async_work implementation relies on pthread too. I have ported libuv's threadpool to emnapi. |
Sorry, something went wrong.
|
I'm still curious about the reason that CallbackScope can not be implemented for missing JavaScript equivalent representatives. IMO, classes like HandleScope, AsyncWorker, and Reference are native structures, not JavaScript values. On the other hand, CallbackScope and TSFN themselves don't create any threads or require threaded works. Instead, AsyncWorker and its subclasses are executed in the thread pool. Would it be more reasonable to only scope AsyncWorker in NAPI_HAS_THREADS instead? |
Sorry, something went wrong.
Imagine this, you call napi_open_callback_scope() in wasm, then wasm need to tell the host (Node.js) open a real node::CallbackScope or call real native napi_open_callback_scope, then return the pointer address to wasm memory to save. wasm napi_open_callback_scope() -> Node.js JavaScript -> call napi_open_callback_scope in native node binding -> return pointer -> JavaScript -> set the pointer address to wasm memory However, both of them failed, Node.js doesn't allow leaving an opening CallbackScope before returning to JavaScript from C++, it just crashed, saying something like callback scope mismatch. In addition, when native callback scope destructing, the pending process.nextTick() will be called before returning back to JavaScript, and there is no such async context in browser, so it can be not implemented on browser.
emnapi's napi_value / napi_reference / napi_handle_scope is not the native, they are implemented in JavaScript, just JavaScript numbers.
I understand your point, but without threads, TSFN and AsyncWorker never work, they are broken. TSFN also rely on pthread mutex. Currently, if no pthread support, napi_create_async_work and such APIs in emnapi will always return napi_generic_failure. Scope them in NAPI_HAS_THREADS can warn user at compile time. |
Sorry, something went wrong.
|
And this PR doesn't affect native platforms, NAPI_HAS_THREADS is always 1 in native. |
Sorry, something went wrong.
Thanks for the explanation! However, HandleScope has the same limitation as CallbackScope: https://github.com/nodejs/node/blob/main/src/js_native_api_v8.h#L95-L96. How did you circumvent the issue for HandleScope, as it is essential for managing javascript value handles? I think CallbackScope can be used without threads. It is essential for any async callbacks to build proper async context tracking. So I'd find it would be great to ensure it is available unconditionally too. |
Sorry, something went wrong.
As I explained above,
HandleScope in emnapi is completely implemented in JavaScript, so is napi_value / napi_reference / napi_deferred, they are independent of native Node.js runtime, all these things are available on browser!
Yeah, I'm scoping Napi::CallbackScope in !defined(__wasm__), not NAPI_HAS_THREADS. Line 2408 in 69300c3 |
Sorry, something went wrong.
In this case, I'd find it is an implementation detail of emnapi. |
Sorry, something went wrong.
|
napi_async_init/napi_async_destroy and napi_open_callback_scope/napi_close_callback_scope should be able to be implemented with JS API AsyncResource. Edit: probably there is a problem with asyncResource.runInAsyncScope. Sorry. |
Sorry, something went wrong.
|
Now I have removed !defined(__wasm__) on CallbackScope. napi_async_init, napi_async_destroy and napi_make_callback have been implemented via node binding as a bridge. https://github.com/toyobayashi/emnapi/blob/main/packages/emnapi/src/node.ts |
Sorry, something went wrong.
CallbackScope cannot be implemented in wasm, this thing itself is not strongly related to emnapi, so I personally prefer to put it in !defined(__wasm__). Or maybe there is probability to implement this in the future if Node.js add some ability to workaround? |
Sorry, something went wrong.
|
Yeah, I'm thinking of providing a built-in wasm node-api module in Node.js since we already allowed declaring a wasm addon in node_api.h. I'll create a tracking issue for this. Previous discussion: nodejs/abi-stable-node#375 |
Sorry, something went wrong.
|
By the way, would it be possible to consider mentioning emnapi in the official documentation Node-API chapter? I have confidence to say now emnapi is the only project which has so complete Node-API implementation for wasm, official test cases guarantees that the runtime behavior on browser is almost exactly the same as on native Node.js. Implementation detail also follows official source code as far as possible. I believe this project can make contribution to community :) |
Sorry, something went wrong.
|
Please feel free to submit a PR on https://github.com/nodejs/abi-stable-node/blob/doc/node-api-engine-bindings.md! |
Sorry, something went wrong.
PR-URL: #46633 Refs: #33597 Refs: nodejs/node-addon-api#1283 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: James M Snell <jasnell@gmail.com>
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
PR-URL: #1283 Reviewed-By: Michael Dawson <midawson@redhat.com Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
PR-URL: #46633 Refs: #33597 Refs: nodejs/node-addon-api#1283 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #46633 Refs: #33597 Refs: nodejs/node-addon-api#1283 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node-addon-api#1283 Reviewed-By: Michael Dawson <midawson@redhat.com Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
| Back | FazBrowse Home | New Git URL |
Hi, Node-API team.
I created emnapi, that is a Node-API implementation for Emscripten/wasi-sdk/wasm32. This project aims to help users port their or existing Node-API native addons to wasm, and run it on browser! It is well tested by using Node.js official test cases. Recently napi-rs is also plan to integrate emnapi so that napi-rs can target wasm as well. See real world use cases and discussions here.
Currently emnapi shipped a modified copy of node-addon-api in @tybys/emnapi npm package, due to the original source disables threadsafe function API when building for wasm, though tsfn can work well by relying on emscripten pthreads. And CallbackScope can not be implemented, neither on browser nor on Node.js. Needless to say in the browser environment, even on Node.js, the reason is that native node::CallbackScope* can not be returned to JavaScript.
This PR defines NAPI_HAS_THREADS to make TSFN available on Emscripten, and disable CallbackScope in wasm. Thus emnapi no longer need to maintain a copy, users can just do npm install node-addon-api to work with emnapi.