src: fix use-after-free in CleanupHookThunkRun - #65630

everett1992 · 2026-08-28T23:51:00Z

This PR is a continuation of @sreehariannam's work in

I've added a regression test and fixed the lint issue. I'm only opening this because there's no activity on that PR, and this bug is blocking the backport of #65042 which fixes a crash with better-sqlite3 on node v24.19.0

Fixes: #65195

codecov · 2026-08-29T18:10:05Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.07%. Comparing base (f9ab994) to head (fd325a7).
⚠️ Report is 30 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #65630   +/-   ##
=======================================
  Coverage   90.07%   90.07%           
=======================================
  Files         754      754           
  Lines      256395   256398    +3     
  Branches    48494    48495    +1     
=======================================
+ Hits       230947   230954    +7     
+ Misses      16563    16561    -2     
+ Partials     8885     8883    -2

Files with missing lines	Coverage Δ
src/api/hooks.cc	`88.19% <100.00%> (-0.41%)`	⬇️

... and 29 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

CleanupHookThunkRun() read thunk->isolate/fun/arg from the CleanupHookThunk after invoking thunk->fun(). For every node::ObjectWrap alive at teardown, thunk->fun is ObjectWrap::CleanupHook, which deletes the wrap; ~ObjectWrap() calls RemoveEnvironmentCleanupHook() itself, erasing the CleanupHookThunk from the registry and freeing the node it lives in. The subsequent read of thunk->isolate/fun/arg to make the (now redundant) second RemoveEnvironmentCleanupHook() call was therefore a use-after-free. Cache the fields before running the hook so nothing is read from `thunk` once it may have been freed. Taken over from nodejs#65196, which has been inactive; the original change is unmodified apart from the added comment. This also unblocks nodejs#65042, the backport of the cleanup hook registry to v24.x. Without that registry ~ObjectWrap() asserts during garbage collection, so every 24.x runtime aborts for ObjectWrap addons (nodejs#65446), as do 26.x runtimes before 26.4.0 when used with newer headers (nodejs#65262). Fixes: nodejs#65195 Refs: nodejs#65196 Refs: nodejs#65042 Refs: nodejs#65446 Refs: nodejs#65262 Assisted-by: a closed-source coding agent Co-authored-by: Sreehari Annam <sreehari.annam@gmail.com> Signed-off-by: Caleb Everett <everett.caleb@gmail.com>

Add a cctest that registers an environment cleanup hook which removes itself while the cleanup queue is drained. It exercises CleanupHookThunkRun(), which must not read the CleanupHookThunk after invoking the hook, because the hook has already erased and freed it. The hook is registered directly rather than through node::ObjectWrap. ObjectWrap is what makes this reachable for addons since nodejs#63642, because its destructor removes its own hook, and nodejs#65195 reproduces the fault that way with test/addons/worker-addon-exit. That reproducer needs an addon build and depends on when the wrapper is collected, whereas this test drives the self-removal directly. The use-after-free is silent in ordinary builds and is caught by the ASan/Valgrind CI, which is how the original assertion (nodejs#63923) surfaced. Verified locally with an ASan build: without the preceding commit both this test and test/addons/worker-addon-exit report heap-use-after-free in CleanupHookThunkRun(); both are clean with it. Refs: nodejs#65195 Refs: nodejs#65196 Assisted-by: a closed-source coding agent Co-authored-by: Sreehari Annam <sreehari.annam@gmail.com> Co-authored-by: nsavoire <19255994+nsavoire@users.noreply.github.com> Signed-off-by: Caleb Everett <everett.caleb@gmail.com>

nodejs-github-bot · 2026-09-02T05:32:59Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76871/

…53390) * fix: backport Node.js cleanup hook fixes for node::ObjectWrap addons Node.js 24.19.0 made node::ObjectWrap register an environment cleanup hook but did not carry the fix that makes RemoveEnvironmentCleanupHook() safe to call from ~ObjectWrap() during garbage collection, so any addon built against the 24.19.0+ headers aborts with "Assertion failed: (env) != nullptr" once one of its wrapped objects is collected while no context is entered. Cherry-picks: * nodejs/node#63985 "src: keep global list of addon-provided cleanup hooks" (68321eff80918e324e9fdaf1aa8cee6db14b84f7 on nodejs/node main). * nodejs/node#65630 "src: fix use-after-free in CleanupHookThunkRun" at dd2fa31b1d99e5a6710e08ce4139da7c2e344598 and its regression cctest at fd325a7fc2b33839c024220aa0c44b9104749686 (not yet landed upstream). Fixes: #53387 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: add node::ObjectWrap native addon GC regression spec Adds a minimal node::ObjectWrap addon fixture with a helper that runs a full garbage collection while no context is entered, and a spec that loads it in an ELECTRON_RUN_AS_NODE child process and asserts the process exits cleanly. Reproduces #53387 on unfixed builds. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: rebuild native addon fixtures against Electron's node headers The native addon fixtures under spec/fixtures/native-addon are yarn workspaces, so yarn only runs their build scripts once per install state. In CI the root `yarn install` builds them first, against the system Node.js headers, and the later spec install with npm_config_nodedir set never rebuilds them. N-API fixtures load anyway, but the object-wrap fixture uses Node's C++ API, embeds the system NODE_MODULE_VERSION, and is rejected by Electron with ERR_DLOPEN_FAILED. Rebuild every native addon fixture with node-gyp after the spec install so they are compiled against the configured Electron headers, which is what spec-runner already intended. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: build C++ API addon fixtures with the Chromium toolchain on Linux The rebuild of the native addon fixtures against Electron's node headers broke every Linux test shard: the CI test image ships GCC 11, which cannot parse the V8 headers (`class V8_DEPRECATE_SOON(...) V8_EXPORT Value` mixes a C++11 attribute with a GNU attribute, rejected by GCC <= 12, see #53284): v8config.h:865:20: error: expected identifier before '__attribute__' v8-primitive.h:658:4: error: expected '}' before 'public' Only addons registering through Node's C++ module API (NODE_MODULE / NODE_MODULE_INIT) embed NODE_MODULE_VERSION and need Electron's headers, so limit the rebuild to those; the N-API fixtures keep loading fine as built by the root install. On Linux, when the Chromium toolchain from the source checkout is present (CI restores third_party/llvm-build and libc++ into the test job via src_artifacts, and the nan tests already build with it), compile the fixture with clang and Electron's libc++ exactly as script/nan-spec-runner.js does; otherwise, and for the --electronVersion prebuilt path, keep using the system compiler. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: make the Electron-header fixture rebuild opt-in and fix it on Windows The Windows test shards failed the fixture rebuild before configure: node-gyp inherits NPM_CONFIG_MSVS_VERSION=2022 from the test step's env, but the runners only ship a newer Visual Studio ("msvs_version does not match this version"). The root install that built the same fixtures ran without that pin and picked the installed toolset, so strip it from the rebuild environment and let node-gyp detect Visual Studio. The Electron import library it links against is already staged by script/actions/move-artifacts.sh as gen/node_headers/Release/node.lib. Also replace the source scan for NODE_MODULE usage with an explicit "electron:requiresElectronHeaders" flag in the fixture's package.json, so only fixtures that declare they need Electron's headers (currently object-wrap) are rebuilt and the N-API fixtures are left untouched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: cope with hosts that cannot build the Electron-header addon fixture The rebuild of spec/fixtures/native-addon/object-wrap against Electron's headers broke two more sets of Linux test shards: * linux-arm64: the test job runs on an arm64 host but restores the x64 Chromium toolchain of the cross-compiling build job, so node-gyp died with "Exec format error: .../llvm-build/Release+Asserts/bin/clang". The image's GCC is the one that rejects the V8 headers (#53284), so nothing on that host can compile the fixture. Check that the Chromium clang actually runs before selecting it; when it does not, warn loudly, skip the rebuild and export ELECTRON_SKIP_ELECTRON_HEADER_ADDON_SPECS so the ObjectWrap spec is reported as skipped there (with a comment pointing at this gap) instead of failing with an ABI error. Every other platform keeps running the spec for real. * linux-x64-asan / -ubsan: the link failed with "ld.lld: error: unable to find library -lc++abi". Sanitizer builds compile libc++abi into the electron executable and export it from there (export_libcxxabi_from_executables) rather than producing libc++abi.a, so only pass -lc++abi when that archive exists; the symbols resolve from the executable when the addon is loaded. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: skip the Electron-header addon specs when the spec install is skipped The linux-arm test job runs the specs with --skipYarnInstall, so the spec install that rebuilds spec/fixtures/native-addon/object-wrap against Electron's headers never happens there and the ObjectWrap spec failed to load the fixture the root install built against the system Node.js headers ("was compiled against a different Node.js version using NODE_MODULE_VERSION 127. This version of Node.js requires NODE_MODULE_VERSION 146"). When the install is needed but skipped, warn and export ELECTRON_SKIP_ELECTRON_HEADER_ADDON_SPECS so those specs are reported as skipped, as on linux-arm64 where no toolchain can build the fixture. Also format the previous change with oxfmt, which `yarn lint:fmt` flagged in CI. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U --------- Co-authored-by: Claude <noreply@anthropic.com>

…(44-x-y) (#53392) * fix: backport Node.js cleanup hook fixes for node::ObjectWrap addons Node.js 24.19.0 made node::ObjectWrap register an environment cleanup hook but did not carry the fix that makes RemoveEnvironmentCleanupHook() safe to call from ~ObjectWrap() during garbage collection, so any addon built against the 24.19.0+ headers aborts with "Assertion failed: (env) != nullptr" once one of its wrapped objects is collected while no context is entered. Cherry-picks: * nodejs/node#63985 "src: keep global list of addon-provided cleanup hooks" (68321eff80918e324e9fdaf1aa8cee6db14b84f7 on nodejs/node main). * nodejs/node#65630 "src: fix use-after-free in CleanupHookThunkRun" at dd2fa31b1d99e5a6710e08ce4139da7c2e344598 and its regression cctest at fd325a7fc2b33839c024220aa0c44b9104749686 (not yet landed upstream). Fixes: #53387 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: add node::ObjectWrap native addon GC regression spec Adds a minimal node::ObjectWrap addon fixture with a helper that runs a full garbage collection while no context is entered, and a spec that loads it in an ELECTRON_RUN_AS_NODE child process and asserts the process exits cleanly. Reproduces #53387 on unfixed builds. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: rebuild native addon fixtures against Electron's node headers The native addon fixtures under spec/fixtures/native-addon are yarn workspaces, so yarn only runs their build scripts once per install state. In CI the root `yarn install` builds them first, against the system Node.js headers, and the later spec install with npm_config_nodedir set never rebuilds them. N-API fixtures load anyway, but the object-wrap fixture uses Node's C++ API, embeds the system NODE_MODULE_VERSION, and is rejected by Electron with ERR_DLOPEN_FAILED. Rebuild every native addon fixture with node-gyp after the spec install so they are compiled against the configured Electron headers, which is what spec-runner already intended. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: build C++ API addon fixtures with the Chromium toolchain on Linux The rebuild of the native addon fixtures against Electron's node headers broke every Linux test shard: the CI test image ships GCC 11, which cannot parse the V8 headers (`class V8_DEPRECATE_SOON(...) V8_EXPORT Value` mixes a C++11 attribute with a GNU attribute, rejected by GCC <= 12, see #53284): v8config.h:865:20: error: expected identifier before '__attribute__' v8-primitive.h:658:4: error: expected '}' before 'public' Only addons registering through Node's C++ module API (NODE_MODULE / NODE_MODULE_INIT) embed NODE_MODULE_VERSION and need Electron's headers, so limit the rebuild to those; the N-API fixtures keep loading fine as built by the root install. On Linux, when the Chromium toolchain from the source checkout is present (CI restores third_party/llvm-build and libc++ into the test job via src_artifacts, and the nan tests already build with it), compile the fixture with clang and Electron's libc++ exactly as script/nan-spec-runner.js does; otherwise, and for the --electronVersion prebuilt path, keep using the system compiler. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: make the Electron-header fixture rebuild opt-in and fix it on Windows The Windows test shards failed the fixture rebuild before configure: node-gyp inherits NPM_CONFIG_MSVS_VERSION=2022 from the test step's env, but the runners only ship a newer Visual Studio ("msvs_version does not match this version"). The root install that built the same fixtures ran without that pin and picked the installed toolset, so strip it from the rebuild environment and let node-gyp detect Visual Studio. The Electron import library it links against is already staged by script/actions/move-artifacts.sh as gen/node_headers/Release/node.lib. Also replace the source scan for NODE_MODULE usage with an explicit "electron:requiresElectronHeaders" flag in the fixture's package.json, so only fixtures that declare they need Electron's headers (currently object-wrap) are rebuilt and the N-API fixtures are left untouched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: cope with hosts that cannot build the Electron-header addon fixture The rebuild of spec/fixtures/native-addon/object-wrap against Electron's headers broke two more sets of Linux test shards: * linux-arm64: the test job runs on an arm64 host but restores the x64 Chromium toolchain of the cross-compiling build job, so node-gyp died with "Exec format error: .../llvm-build/Release+Asserts/bin/clang". The image's GCC is the one that rejects the V8 headers (#53284), so nothing on that host can compile the fixture. Check that the Chromium clang actually runs before selecting it; when it does not, warn loudly, skip the rebuild and export ELECTRON_SKIP_ELECTRON_HEADER_ADDON_SPECS so the ObjectWrap spec is reported as skipped there (with a comment pointing at this gap) instead of failing with an ABI error. Every other platform keeps running the spec for real. * linux-x64-asan / -ubsan: the link failed with "ld.lld: error: unable to find library -lc++abi". Sanitizer builds compile libc++abi into the electron executable and export it from there (export_libcxxabi_from_executables) rather than producing libc++abi.a, so only pass -lc++abi when that archive exists; the symbols resolve from the executable when the addon is loaded. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U (cherry picked from commit a723f64) * test: skip the Electron-header addon specs when the spec install is skipped The linux-arm test job runs the specs with --skipYarnInstall, so the spec install that rebuilds spec/fixtures/native-addon/object-wrap against Electron's headers never happens there and the ObjectWrap spec failed to load the fixture the root install built against the system Node.js headers ("was compiled against a different Node.js version using NODE_MODULE_VERSION 127. This version of Node.js requires NODE_MODULE_VERSION 146"). When the install is needed but skipped, warn and export ELECTRON_SKIP_ELECTRON_HEADER_ADDON_SPECS so those specs are reported as skipped, as on linux-arm64 where no toolchain can build the fixture. Also format the previous change with oxfmt, which `yarn lint:fmt` flagged in CI. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U (cherry picked from commit de4cc41) --------- Co-authored-by: Claude <noreply@anthropic.com>

…(43-x-y) (#53393) * fix: backport Node.js cleanup hook fixes for node::ObjectWrap addons Node.js 24.19.0 made node::ObjectWrap register an environment cleanup hook but did not carry the fix that makes RemoveEnvironmentCleanupHook() safe to call from ~ObjectWrap() during garbage collection, so any addon built against the 24.19.0+ headers aborts with "Assertion failed: (env) != nullptr" once one of its wrapped objects is collected while no context is entered. Cherry-picks: * nodejs/node#63985 "src: keep global list of addon-provided cleanup hooks" (68321eff80918e324e9fdaf1aa8cee6db14b84f7 on nodejs/node main). * nodejs/node#65630 "src: fix use-after-free in CleanupHookThunkRun" at dd2fa31b1d99e5a6710e08ce4139da7c2e344598 and its regression cctest at fd325a7fc2b33839c024220aa0c44b9104749686 (not yet landed upstream). Fixes: #53387 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: add node::ObjectWrap native addon GC regression spec Adds a minimal node::ObjectWrap addon fixture with a helper that runs a full garbage collection while no context is entered, and a spec that loads it in an ELECTRON_RUN_AS_NODE child process and asserts the process exits cleanly. Reproduces #53387 on unfixed builds. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: rebuild native addon fixtures against Electron's node headers The native addon fixtures under spec/fixtures/native-addon are yarn workspaces, so yarn only runs their build scripts once per install state. In CI the root `yarn install` builds them first, against the system Node.js headers, and the later spec install with npm_config_nodedir set never rebuilds them. N-API fixtures load anyway, but the object-wrap fixture uses Node's C++ API, embeds the system NODE_MODULE_VERSION, and is rejected by Electron with ERR_DLOPEN_FAILED. Rebuild every native addon fixture with node-gyp after the spec install so they are compiled against the configured Electron headers, which is what spec-runner already intended. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: build C++ API addon fixtures with the Chromium toolchain on Linux The rebuild of the native addon fixtures against Electron's node headers broke every Linux test shard: the CI test image ships GCC 11, which cannot parse the V8 headers (`class V8_DEPRECATE_SOON(...) V8_EXPORT Value` mixes a C++11 attribute with a GNU attribute, rejected by GCC <= 12, see #53284): v8config.h:865:20: error: expected identifier before '__attribute__' v8-primitive.h:658:4: error: expected '}' before 'public' Only addons registering through Node's C++ module API (NODE_MODULE / NODE_MODULE_INIT) embed NODE_MODULE_VERSION and need Electron's headers, so limit the rebuild to those; the N-API fixtures keep loading fine as built by the root install. On Linux, when the Chromium toolchain from the source checkout is present (CI restores third_party/llvm-build and libc++ into the test job via src_artifacts, and the nan tests already build with it), compile the fixture with clang and Electron's libc++ exactly as script/nan-spec-runner.js does; otherwise, and for the --electronVersion prebuilt path, keep using the system compiler. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: make the Electron-header fixture rebuild opt-in and fix it on Windows The Windows test shards failed the fixture rebuild before configure: node-gyp inherits NPM_CONFIG_MSVS_VERSION=2022 from the test step's env, but the runners only ship a newer Visual Studio ("msvs_version does not match this version"). The root install that built the same fixtures ran without that pin and picked the installed toolset, so strip it from the rebuild environment and let node-gyp detect Visual Studio. The Electron import library it links against is already staged by script/actions/move-artifacts.sh as gen/node_headers/Release/node.lib. Also replace the source scan for NODE_MODULE usage with an explicit "electron:requiresElectronHeaders" flag in the fixture's package.json, so only fixtures that declare they need Electron's headers (currently object-wrap) are rebuilt and the N-API fixtures are left untouched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: cope with hosts that cannot build the Electron-header addon fixture The rebuild of spec/fixtures/native-addon/object-wrap against Electron's headers broke two more sets of Linux test shards: * linux-arm64: the test job runs on an arm64 host but restores the x64 Chromium toolchain of the cross-compiling build job, so node-gyp died with "Exec format error: .../llvm-build/Release+Asserts/bin/clang". The image's GCC is the one that rejects the V8 headers (#53284), so nothing on that host can compile the fixture. Check that the Chromium clang actually runs before selecting it; when it does not, warn loudly, skip the rebuild and export ELECTRON_SKIP_ELECTRON_HEADER_ADDON_SPECS so the ObjectWrap spec is reported as skipped there (with a comment pointing at this gap) instead of failing with an ABI error. Every other platform keeps running the spec for real. * linux-x64-asan / -ubsan: the link failed with "ld.lld: error: unable to find library -lc++abi". Sanitizer builds compile libc++abi into the electron executable and export it from there (export_libcxxabi_from_executables) rather than producing libc++abi.a, so only pass -lc++abi when that archive exists; the symbols resolve from the executable when the addon is loaded. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U (cherry picked from commit a723f64) * test: skip the Electron-header addon specs when the spec install is skipped The linux-arm test job runs the specs with --skipYarnInstall, so the spec install that rebuilds spec/fixtures/native-addon/object-wrap against Electron's headers never happens there and the ObjectWrap spec failed to load the fixture the root install built against the system Node.js headers ("was compiled against a different Node.js version using NODE_MODULE_VERSION 127. This version of Node.js requires NODE_MODULE_VERSION 146"). When the install is needed but skipped, warn and export ELECTRON_SKIP_ELECTRON_HEADER_ADDON_SPECS so those specs are reported as skipped, as on linux-arm64 where no toolchain can build the fixture. Also format the previous change with oxfmt, which `yarn lint:fmt` flagged in CI. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U (cherry picked from commit de4cc41) --------- Co-authored-by: Claude <noreply@anthropic.com>

…(42-x-y) (#53394) * fix: backport Node.js cleanup hook fixes for node::ObjectWrap addons Node.js 24.19.0 made node::ObjectWrap register an environment cleanup hook but did not carry the fix that makes RemoveEnvironmentCleanupHook() safe to call from ~ObjectWrap() during garbage collection, so any addon built against the 24.19.0+ headers aborts with "Assertion failed: (env) != nullptr" once one of its wrapped objects is collected while no context is entered. Cherry-picks: * nodejs/node#63985 "src: keep global list of addon-provided cleanup hooks" (68321eff80918e324e9fdaf1aa8cee6db14b84f7 on nodejs/node main). * nodejs/node#65630 "src: fix use-after-free in CleanupHookThunkRun" at dd2fa31b1d99e5a6710e08ce4139da7c2e344598 and its regression cctest at fd325a7fc2b33839c024220aa0c44b9104749686 (not yet landed upstream). Fixes: #53387 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: add node::ObjectWrap native addon GC regression spec Adds a minimal node::ObjectWrap addon fixture with a helper that runs a full garbage collection while no context is entered, and a spec that loads it in an ELECTRON_RUN_AS_NODE child process and asserts the process exits cleanly. Reproduces #53387 on unfixed builds. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: rebuild native addon fixtures against Electron's node headers The native addon fixtures under spec/fixtures/native-addon are yarn workspaces, so yarn only runs their build scripts once per install state. In CI the root `yarn install` builds them first, against the system Node.js headers, and the later spec install with npm_config_nodedir set never rebuilds them. N-API fixtures load anyway, but the object-wrap fixture uses Node's C++ API, embeds the system NODE_MODULE_VERSION, and is rejected by Electron with ERR_DLOPEN_FAILED. Rebuild every native addon fixture with node-gyp after the spec install so they are compiled against the configured Electron headers, which is what spec-runner already intended. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: build C++ API addon fixtures with the Chromium toolchain on Linux The rebuild of the native addon fixtures against Electron's node headers broke every Linux test shard: the CI test image ships GCC 11, which cannot parse the V8 headers (`class V8_DEPRECATE_SOON(...) V8_EXPORT Value` mixes a C++11 attribute with a GNU attribute, rejected by GCC <= 12, see #53284): v8config.h:865:20: error: expected identifier before '__attribute__' v8-primitive.h:658:4: error: expected '}' before 'public' Only addons registering through Node's C++ module API (NODE_MODULE / NODE_MODULE_INIT) embed NODE_MODULE_VERSION and need Electron's headers, so limit the rebuild to those; the N-API fixtures keep loading fine as built by the root install. On Linux, when the Chromium toolchain from the source checkout is present (CI restores third_party/llvm-build and libc++ into the test job via src_artifacts, and the nan tests already build with it), compile the fixture with clang and Electron's libc++ exactly as script/nan-spec-runner.js does; otherwise, and for the --electronVersion prebuilt path, keep using the system compiler. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: make the Electron-header fixture rebuild opt-in and fix it on Windows The Windows test shards failed the fixture rebuild before configure: node-gyp inherits NPM_CONFIG_MSVS_VERSION=2022 from the test step's env, but the runners only ship a newer Visual Studio ("msvs_version does not match this version"). The root install that built the same fixtures ran without that pin and picked the installed toolset, so strip it from the rebuild environment and let node-gyp detect Visual Studio. The Electron import library it links against is already staged by script/actions/move-artifacts.sh as gen/node_headers/Release/node.lib. Also replace the source scan for NODE_MODULE usage with an explicit "electron:requiresElectronHeaders" flag in the fixture's package.json, so only fixtures that declare they need Electron's headers (currently object-wrap) are rebuilt and the N-API fixtures are left untouched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: cope with hosts that cannot build the Electron-header addon fixture The rebuild of spec/fixtures/native-addon/object-wrap against Electron's headers broke two more sets of Linux test shards: * linux-arm64: the test job runs on an arm64 host but restores the x64 Chromium toolchain of the cross-compiling build job, so node-gyp died with "Exec format error: .../llvm-build/Release+Asserts/bin/clang". The image's GCC is the one that rejects the V8 headers (#53284), so nothing on that host can compile the fixture. Check that the Chromium clang actually runs before selecting it; when it does not, warn loudly, skip the rebuild and export ELECTRON_SKIP_ELECTRON_HEADER_ADDON_SPECS so the ObjectWrap spec is reported as skipped there (with a comment pointing at this gap) instead of failing with an ABI error. Every other platform keeps running the spec for real. * linux-x64-asan / -ubsan: the link failed with "ld.lld: error: unable to find library -lc++abi". Sanitizer builds compile libc++abi into the electron executable and export it from there (export_libcxxabi_from_executables) rather than producing libc++abi.a, so only pass -lc++abi when that archive exists; the symbols resolve from the executable when the addon is loaded. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U (cherry picked from commit a723f64) * test: skip the Electron-header addon specs when the spec install is skipped The linux-arm test job runs the specs with --skipYarnInstall, so the spec install that rebuilds spec/fixtures/native-addon/object-wrap against Electron's headers never happens there and the ObjectWrap spec failed to load the fixture the root install built against the system Node.js headers ("was compiled against a different Node.js version using NODE_MODULE_VERSION 127. This version of Node.js requires NODE_MODULE_VERSION 146"). When the install is needed but skipped, warn and export ELECTRON_SKIP_ELECTRON_HEADER_ADDON_SPECS so those specs are reported as skipped, as on linux-arm64 where no toolchain can build the fixture. Also format the previous change with oxfmt, which `yarn lint:fmt` flagged in CI. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U (cherry picked from commit de4cc41) --------- Co-authored-by: Claude <noreply@anthropic.com>

…(45-x-y) (#53391) * fix: backport Node.js cleanup hook fixes for node::ObjectWrap addons Node.js 24.19.0 made node::ObjectWrap register an environment cleanup hook but did not carry the fix that makes RemoveEnvironmentCleanupHook() safe to call from ~ObjectWrap() during garbage collection, so any addon built against the 24.19.0+ headers aborts with "Assertion failed: (env) != nullptr" once one of its wrapped objects is collected while no context is entered. Cherry-picks: * nodejs/node#63985 "src: keep global list of addon-provided cleanup hooks" (68321eff80918e324e9fdaf1aa8cee6db14b84f7 on nodejs/node main). * nodejs/node#65630 "src: fix use-after-free in CleanupHookThunkRun" at dd2fa31b1d99e5a6710e08ce4139da7c2e344598 and its regression cctest at fd325a7fc2b33839c024220aa0c44b9104749686 (not yet landed upstream). Fixes: #53387 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: add node::ObjectWrap native addon GC regression spec Adds a minimal node::ObjectWrap addon fixture with a helper that runs a full garbage collection while no context is entered, and a spec that loads it in an ELECTRON_RUN_AS_NODE child process and asserts the process exits cleanly. Reproduces #53387 on unfixed builds. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: rebuild native addon fixtures against Electron's node headers The native addon fixtures under spec/fixtures/native-addon are yarn workspaces, so yarn only runs their build scripts once per install state. In CI the root `yarn install` builds them first, against the system Node.js headers, and the later spec install with npm_config_nodedir set never rebuilds them. N-API fixtures load anyway, but the object-wrap fixture uses Node's C++ API, embeds the system NODE_MODULE_VERSION, and is rejected by Electron with ERR_DLOPEN_FAILED. Rebuild every native addon fixture with node-gyp after the spec install so they are compiled against the configured Electron headers, which is what spec-runner already intended. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: build C++ API addon fixtures with the Chromium toolchain on Linux The rebuild of the native addon fixtures against Electron's node headers broke every Linux test shard: the CI test image ships GCC 11, which cannot parse the V8 headers (`class V8_DEPRECATE_SOON(...) V8_EXPORT Value` mixes a C++11 attribute with a GNU attribute, rejected by GCC <= 12, see #53284): v8config.h:865:20: error: expected identifier before '__attribute__' v8-primitive.h:658:4: error: expected '}' before 'public' Only addons registering through Node's C++ module API (NODE_MODULE / NODE_MODULE_INIT) embed NODE_MODULE_VERSION and need Electron's headers, so limit the rebuild to those; the N-API fixtures keep loading fine as built by the root install. On Linux, when the Chromium toolchain from the source checkout is present (CI restores third_party/llvm-build and libc++ into the test job via src_artifacts, and the nan tests already build with it), compile the fixture with clang and Electron's libc++ exactly as script/nan-spec-runner.js does; otherwise, and for the --electronVersion prebuilt path, keep using the system compiler. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: make the Electron-header fixture rebuild opt-in and fix it on Windows The Windows test shards failed the fixture rebuild before configure: node-gyp inherits NPM_CONFIG_MSVS_VERSION=2022 from the test step's env, but the runners only ship a newer Visual Studio ("msvs_version does not match this version"). The root install that built the same fixtures ran without that pin and picked the installed toolset, so strip it from the rebuild environment and let node-gyp detect Visual Studio. The Electron import library it links against is already staged by script/actions/move-artifacts.sh as gen/node_headers/Release/node.lib. Also replace the source scan for NODE_MODULE usage with an explicit "electron:requiresElectronHeaders" flag in the fixture's package.json, so only fixtures that declare they need Electron's headers (currently object-wrap) are rebuilt and the N-API fixtures are left untouched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U * test: cope with hosts that cannot build the Electron-header addon fixture The rebuild of spec/fixtures/native-addon/object-wrap against Electron's headers broke two more sets of Linux test shards: * linux-arm64: the test job runs on an arm64 host but restores the x64 Chromium toolchain of the cross-compiling build job, so node-gyp died with "Exec format error: .../llvm-build/Release+Asserts/bin/clang". The image's GCC is the one that rejects the V8 headers (#53284), so nothing on that host can compile the fixture. Check that the Chromium clang actually runs before selecting it; when it does not, warn loudly, skip the rebuild and export ELECTRON_SKIP_ELECTRON_HEADER_ADDON_SPECS so the ObjectWrap spec is reported as skipped there (with a comment pointing at this gap) instead of failing with an ABI error. Every other platform keeps running the spec for real. * linux-x64-asan / -ubsan: the link failed with "ld.lld: error: unable to find library -lc++abi". Sanitizer builds compile libc++abi into the electron executable and export it from there (export_libcxxabi_from_executables) rather than producing libc++abi.a, so only pass -lc++abi when that archive exists; the symbols resolve from the executable when the addon is loaded. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U (cherry picked from commit a723f64) * test: skip the Electron-header addon specs when the spec install is skipped The linux-arm test job runs the specs with --skipYarnInstall, so the spec install that rebuilds spec/fixtures/native-addon/object-wrap against Electron's headers never happens there and the ObjectWrap spec failed to load the fixture the root install built against the system Node.js headers ("was compiled against a different Node.js version using NODE_MODULE_VERSION 127. This version of Node.js requires NODE_MODULE_VERSION 146"). When the install is needed but skipped, warn and export ELECTRON_SKIP_ELECTRON_HEADER_ADDON_SPECS so those specs are reported as skipped, as on linux-arm64 where no toolchain can build the fixture. Also format the previous change with oxfmt, which `yarn lint:fmt` flagged in CI. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NawBg2exXYXTXt12SBGe9U (cherry picked from commit de4cc41) --------- Co-authored-by: Claude <noreply@anthropic.com>

nodejs-github-bot · 2026-09-04T16:56:45Z

Landed in 03e2b9b

The registry behind `AddEnvironmentCleanupHook()` is keyed on {isolate, fun, arg} and asserts that every insertion is unique. Two Environments on one isolate that register the same hook, which the Node-API documentation allows per environment, abort the process on the second `napi_add_env_cleanup_hook()`. Key the registry on `arg` only and tell entries apart by Environment: adding the same hook to one Environment twice still aborts as documented, and removal prefers the current Environment's registration, falling back to a matching one from another Environment when there is no current context. Because the entry to remove after a hook has run can no longer be found by {isolate, fun, arg} alone, `CleanupHookThunkRun()` marks its entry as running and erases exactly that entry afterwards; a removal of a running entry (a hook removing itself, as `~ObjectWrap()` does) is a no-op, which keeps the use-after-free fixed by nodejs#65630 fixed. Refs: nodejs#63985 Refs: nodejs#65630 Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Aug 28, 2026

everett1992 marked this pull request as ready for review August 29, 2026 16:32

trivikr changed the title ~~Test cleanup hook uaf regression 65196~~ src: fix use-after-free in CleanupHookThunkRun Aug 31, 2026

trivikr mentioned this pull request Aug 31, 2026

src: fix use-after-free in CleanupHookThunkRun #65196

Closed

trivikr approved these changes Sep 1, 2026

View reviewed changes

trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 1, 2026

trivikr requested a review from aduh95 September 1, 2026 02:05

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 1, 2026

This comment was marked as outdated.

Sign in to view

everett1992 and others added 2 commits September 1, 2026 09:53

everett1992 force-pushed the test-cleanup-hook-uaf-regression-65196 branch from bb42c6d to fd325a7 Compare September 1, 2026 16:55

trivikr approved these changes Sep 1, 2026

View reviewed changes

trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 1, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 1, 2026

This comment was marked as outdated.

Sign in to view

trivikr added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Sep 2, 2026

trivikr added commit-queue-squash PRs the Commit Queue should land as one squashed commit. commit-queue PRs queued for automated landing through the Commit Queue. labels Sep 2, 2026

nodejs-github-bot added the lacks-second-approval Commit Queue PRs awaiting a second collaborator approval or completion of the required wait. label Sep 2, 2026

codebytere approved these changes Sep 4, 2026

View reviewed changes

nodejs-github-bot merged commit 03e2b9b into nodejs:main Sep 4, 2026
84 checks passed

nodejs-github-bot removed commit-queue PRs queued for automated landing through the Commit Queue. lacks-second-approval Commit Queue PRs awaiting a second collaborator approval or completion of the required wait. labels Sep 4, 2026

codebytere mentioned this pull request Sep 4, 2026

src: let Environments on one isolate share a cleanup hook #65777

Open

Navigation Menu

Uh oh!

Uh oh!

src: fix use-after-free in CleanupHookThunkRun - #65630