-
Notifications
You must be signed in to change notification settings - Fork 465
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.
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
src: define NAPI_HAS_THREADS
to make TSFN available on Emscripten
#1283
Conversation
Sorry, I have no idea why |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Toyo, thanks for the work!
I'm wondering if classes like AsyncWorker
should be scoped with NAPI_HAS_THREADS
too?
Because emnapi |
I'm still curious about the reason that On the other hand, |
Imagine this, you call
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
emnapi's
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, |
And this PR doesn't affect native platforms, |
Thanks for the explanation! However, I think |
As I explained above,
Yeah, I'm scoping Line 2408 in 69300c3
|
In this case, I'd find it is an implementation detail of emnapi. |
Edit: probably there is a problem with |
Now I have removed
https://github.com/toyobayashi/emnapi/blob/main/packages/emnapi/src/node.ts |
|
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 Previous discussion: nodejs/abi-stable-node#375 |
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 :) |
Please feel free to submit a PR on https://github.com/nodejs/abi-stable-node/blob/doc/node-api-engine-bindings.md! |
PR-URL: #46633 Refs: #33597 Refs: nodejs/node-addon-api#1283 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: James M Snell <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR-URL: #1283 Reviewed-By: Michael Dawson <[email protected] Reviewed-By: Chengzhong Wu <[email protected]>
Landed in 0b53d88 |
PR-URL: #46633 Refs: #33597 Refs: nodejs/node-addon-api#1283 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #46633 Refs: #33597 Refs: nodejs/node-addon-api#1283 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node-addon-api#1283 Reviewed-By: Michael Dawson <[email protected] Reviewed-By: Chengzhong Wu <[email protected]>
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. AndCallbackScope
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 nativenode::CallbackScope*
can not be returned to JavaScript.This PR defines
NAPI_HAS_THREADS
to make TSFN available on Emscripten, and disableCallbackScope
in wasm. Thus emnapi no longer need to maintain a copy, users can just donpm install node-addon-api
to work with emnapi.