René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"napi_threadsafe_function is very hard to use safely","articleBody":"### Version\n\nv22.10.0\n\n### Platform\n\n```text\nLinux s7 5.15.0-69-generic #76-Ubuntu SMP Fri Mar 17 17:19:29 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux\n```\n\n\n### Subsystem\n\nnode-api\n\n### What steps will reproduce the bug?\n\nThe handle obtained with `napi_create_threadsafe_function` is supposed to be usable by arbitrary threads.\r\nIn particular, the following operations should be safe to call from arbitrary threads (until that thread calls `napi_release_threadsafe_function` or receives `napi_closing` from `napi_call_threadsafe_function`):\r\n`napi_get_threadsafe_function_context`, `napi_call_threadsafe_function`, `napi_acquire_threadsafe_function`, and `napi_release_threadsafe_function`.\r\n\r\nHowever, this is currently not the case.\r\nIn particular, in the face of Node.js environment shutdown there are data races and use-after-frees that occur when threads call any of these functions during or after the cleanup that happens when the Node.js environment shuts down.\r\n\r\nThere are two main issues I already found:\r\n- During finalization, the queue is accessed without holding its mutex [here](https://github.com/nodejs/node/blob/b38e3124862f7623fd8e8f0a9b738065d9296a74/src/node_api.cc#L300)\r\n    - If another thread calls `napi_call_threadsafe_function` concurrently, this leads to a data race on the queue internals [here](https://github.com/nodejs/node/blob/b38e3124862f7623fd8e8f0a9b738065d9296a74/src/node_api.cc#L243)\r\n- At the end of finalization, the whole internal state is just deleted [here](https://github.com/nodejs/node/blob/4f5db8b26d906f1cbe9f6a9ac2028b0f7ad88c91/src/node_api.cc#L303) even if there are still threads holding handles to the tsnf.\r\n    - Afterwards each use of any of the above-mentioned functions is a use-after-free bug\r\n\r\nSee https://github.com/mika-fischer/node-bug-napi-tsfn for a detailed reproduction of the issue on Linux using valgrind\r\n\r\nWhile working around this is technically possible (see [src/run/fixed.hpp](https://github.com/mika-fischer/node-bug-napi-tsfn/blob/main/src/run_fixed.hpp)) it involves attaching a finalizer in order to track the finalization state in an external flag, whose lifetime must be managed via a shared_ptr or similar and all access to the TSFN must be protected by another mutex (or read-write-lock). This makes the whole thing very unergonomic to use and I'm pretty sure nobody will jump through these hoops.\r\n\r\nIt should also be relatively easy to fix and not break API or ABI\n\n### How often does it reproduce? Is there a required condition?\n\nAlways\n\n### What is the expected behavior? Why is that the expected behavior?\n\n- Finalization should lock the mutex to make concurrent calls safe.\r\n- Finalization should put the TSFN into a state where all its resources are released, but *only* delete the actual TSFN object if there are no more handles to it. Otherwise the actual deletion should be deferred until one of `napi_release_threadsafe_function` or `napi_call_threadsafe_function` decreases the thread_count to zero.\r\n- In this finalized-but-not-yet deleted state, the operations mentioned above should work as follows:\r\n  - `napi_get_threadsafe_function_context` should return the stored context pointer as usual\r\n  - `napi_call_threadsafe_function` should return `napi_closing`, decrease the thread_count and if it falls to zero delete the TSFN\r\n  - `napi_acquire_threadsafe_function` should return `napi_closing`\r\n  - `napi_release_threadsafe_function` should return `napi_ok`, decrease the thread_count and if it falls to zero delete the TSFN\n\n### What do you see instead?\n\ndata races \u0026 use-after-frees leading to crashes\n\n### Additional information\n\n_No response_","author":{"url":"https://github.com/mika-fischer","@type":"Person","name":"mika-fischer"},"datePublished":"2024-11-03T17:22:10.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":6},"url":"https://github.com/55706/node/issues/55706"}