fix(napi): Implement napi_threadsafe_function ref and unref
#17304
Conversation
|
AFAIU ref/unref do the same thing as release/aquire functions but the later also consumes/takes ownership of the tsfn handle. https://nodejs.org/api/n-api.html#napi_unref_threadsafe_function |
cli/napi/threadsafe_functions.rs
Outdated
| self | ||
| .tsfn_sender | ||
| .unbounded_send(ThreadSafeFunctionStatus::Alive) | ||
| .map_err(|_| Error::GenericFailure)?; | ||
| Ok(()) | ||
| } | ||
|
|
||
| pub fn unref(&mut self) -> Result { | ||
| self | ||
| .tsfn_sender | ||
| .unbounded_send(ThreadSafeFunctionStatus::Dead) | ||
| .map_err(|_| Error::GenericFailure)?; |
There was a problem hiding this comment.
This is brittle and not idempotent - calling unref() multiple times will make NapiState.active_threadsafe_functions go negative. I was looking into fixing this and I believe the most bulletproof solution would either be implementing something similar to what @aapoalas did for FFI functions, or make TsFn have ID and store a hashmap of TsFn mappings in NapiState that keeps track of reference counts. WDYT?
In Node |
Thanks for pointing this out, we discussed this offline earlier today and decided that this integration needs to be mostly rewritten. I'll look into that. |
bartlomieju
changed the title
napi_threadsafe_function ref and unrefnapi_threadsafe_function ref and unref
Co-authored-by: Bartek Iwańczuk <[email protected]>
Co-authored-by: Bartek Iwańczuk <[email protected]>
…denoland#17304)" This reverts commit dd2829b.
Closes #17288