fix(ext/napi): add uv_cond_* polyfills for native addons#35536
Merged
Conversation
Native addons that link against libuv directly (e.g. `@sap/hana-client`) use libuv's condition variables, but the host binary only exported `uv_mutex_*`, `uv_sem_*` and `uv_thread_*`. Loading such an addon failed with `undefined symbol: uv_cond_init`. Add `uv_cond_init`, `uv_cond_destroy`, `uv_cond_signal`, `uv_cond_broadcast`, `uv_cond_wait` and `uv_cond_timedwait`, following the existing semaphore/thread approach: the opaque, platform-specific `uv_cond_t` allocated by the addon stores a small token while the real state lives in a process-global registry. The condvar is backed by its own internal mutex (not the addon's `uv_mutex_t`) that is held across the addon-mutex release, so a concurrent signal can't be lost. The new symbols are added to `symbol_exports.json` and the generated `.def` lists so they end up in the binary's dynamic symbol table. Closes denoland#34736
…to e2e uv_cond_timedwait hardcoded -4039 for the timeout case, but that is only libuv's fallback for Windows (and platforms without ETIMEDOUT). On unix UV_ETIMEDOUT is -ETIMEDOUT (-110 on Linux, -60 on macOS), so an addon comparing the return against its own UV_ETIMEDOUT never matched and misread a timeout. Derive the value per platform from libc, mirroring libuv's uv/errno.h. Drop the in-process unit test for the cond polyfills in favor of the end-to-end napi addon test, which resolves the symbols from the host deno binary at runtime (the thing this change actually ships). The e2e test now asserts the exact platform UV_ETIMEDOUT rather than just a non-zero code, so it would catch the value regression. The cheap size-fits assertion stays in the unit sizes() test.
…ters The existing uv_cond e2e test only exercised uv_cond_signal with a single waiter and the timedwait timeout. uv_cond_broadcast and the multiple-waiter path had no coverage. Add test_uv_cond_broadcast: several worker threads park in uv_cond_wait on the same condition variable, and once all are parked the main thread wakes every one of them with a single uv_cond_broadcast, asserting all workers ran.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #34736.
Native addons that link against libuv directly (e.g.
@sap/hana-client) use libuv's condition variables. The host binary only exporteduv_mutex_*,uv_sem_*anduv_thread_*, so loading such an addon failed:This adds the missing condition-variable symbols:
uv_cond_init,uv_cond_destroy,uv_cond_signal,uv_cond_broadcast,uv_cond_wait,uv_cond_timedwait.They follow the existing
uv_sem_*/uv_thread_*approach inext/napi/uv.rs: the opaque, platform-specificuv_cond_tthe addon allocates only stores a small token, and the real state lives in a process-global registry, so we don't depend on its layout (au32fits the smallestuv_cond_t, Windows' pointer-sizedCONDITION_VARIABLE).The condvar is backed by its own internal mutex rather than the addon's
uv_mutex_t.uv_cond_waittakes the internal mutex, releases the addon mutex, then blocks on the internal mutex; a concurrentuv_cond_signal/uv_cond_broadcastmust take that same internal mutex to notify and can't acquire it until the waiter is parked, so no wakeup is lost.uv_cond_timedwaitreturnsUV_ETIMEDOUT(-4039) on timeout.The new symbols are added to
symbol_exports.jsonand the regenerated.deflists so they land in the binary's dynamic symbol table.Tests
ext/napi/uv.rsdrivinguv_cond_*with a worker thread +uv_mutex, plus a size-fits assertion for the token.tests/napi/src/uv.rs+uv_test.js): a real.nodeaddon resolves the symbols from the hostdenoat runtime and a worker signals a condition variable the main thread waits on. Verified passing locally.