src: add cleanup hooks to node::ObjectWrap

[addaleax]

In two commits to keep attribution to @mohd-akram's PR intact. If somebody feels strongly that they should be a single commit, I can turn them into one with a Co-authored-by tag.

src: add cleanup hooks to node::ObjectWrap

Add cleanup hooks to make sure that addons making use
of this legacy API can do so safely in Worker threads
or embedder-controlled Node.js instances.

Refs: #63575
Fixes: #63540

test: add regression test for using ObjectWrap in worker

This content is taken from #63575.
The original commit message is preserved below:

worker: fix premature addon unload

Weak callbacks could run after addons were unloaded, leading to a crash.

[nodejs-github-bot]

Review requested:

• @nodejs/gyp

[legendecas]

Is this necessary to test with the CleanupHook? I don't think it is necessary to trigger GC here, right?

[addaleax]

You're right, GC would prevent the test case from working, if anything – done in fcb0a4d

[mohd-akram]

@legendecas @addaleax It's not meant to trigger a GC immediately. The purpose of the loop is to make sure the WeakCallback runs, which it does after the addon is unloaded, and therefore causes a segfault absent a fix. Without that loop you won't be testing the visible effects of the clean up hook (i.e. that it removes the WeakCallback). The loop iterations were chosen such that this happens consistently.

[addaleax]

@mohd-akram If the weak callback runs here, won't that mean that the object is destroyed before the Environment and therefore the issue in #63540 will actually not occur? I can reproduce a fairly consistent crash with current Node.js versions (i.e. before this PR) when the loop is commented out and no consistent crash when it is active.

[mohd-akram]

If the weak callback runs here, won't that mean that the object is destroyed before the Environment and therefore the issue in #63540 will actually not occur?

On my system, it doesn't run there, it runs after.

I can reproduce a fairly consistent crash with current Node.js versions (i.e. before this PR) when the loop is commented out and no consistent crash when it is active.

Hmm, that's surprising. Definitely the opposite happening for me:

$ cat test.js
'use strict';
const common = require('../../common');
const { isMainThread, Worker } = require('worker_threads');

if (!isMainThread) {
  const addon = require(`./build/${common.buildType}/binding`);

  // Create some garbage
  const arr = [];
  for (let i = 0; i < 1e5; i++) arr.push(`${i}`.repeat(100));

  new addon.MyObject(10);

  // Should not segfault
  throw new Error('exit');
} else {
  const worker = new Worker(__filename);
  worker.on('error', () => {});
}
$ node --version
v26.2.0
$ node test.js
zsh: segmentation fault  node test.js

What OS are you on?

[addaleax]

It's arm64 macOS.

Testing out a few different combinations: Node.js 26.0.0 has this crash only without the extra array, Node.js 26.1.0 and 26.2.0 do not have it at all, and Node.js @ 40dc5a1 (base of this PR) has it only with the extra array.

I could probably dig deeper into the exact reasons, but regardless of what the outcome of that would be, I don't really see a good reason not to just include both versions as tests, so I've done that in 0cede86

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.32%. Comparing base (40dc5a1) to head (fa40d04).
⚠️ Report is 73 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #63642      +/-   ##
==========================================
+ Coverage   90.30%   90.32%   +0.01%     
==========================================
  Files         730      732       +2     
  Lines      234802   236435    +1633     
  Branches    43957    44531     +574     
==========================================
+ Hits       212041   213560    +1519     
- Misses      14485    14596     +111     
- Partials     8276     8279       +3     

see 58 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.

[nodejs-github-bot]

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

[nodejs-github-bot]

Landed in 3518f93...be7ea27

[joyeecheung]

https://github.com/nodejs/node/actions/runs/27140598797/job/80104749457
https://github.com/nodejs/node/actions/runs/27147175919/job/80128518308

test/addons/worker-addon-exit/test.js has started to fail in unrelated PRs in the github action. Looking at the stack trace it's likely related to this PR?

#  out/Release/node[100828]: void node::RemoveEnvironmentCleanupHook(v8::Isolate*, CleanupHook, void*) at ../../src/api/hooks.cc:130
  #  Assertion failed: (env) != nullptr

----- Native stack trace -----

 1: 0xab6061e07878 node::DumpNativeBacktrace(_IO_FILE*) [out/Release/node]
 2: 0xab6061fc362c node::Assert(node::AssertionInfo const&) [out/Release/node]
 3: 0xab6061d31fa8 node::RemoveEnvironmentCleanupHook(v8::Isolate*, void (*)(void*), void*) [out/Release/node]
 4: 0xff2260822338 MyObject::~MyObject() [/home/runner/work/_temp/node-v27.0.0-nightly2026-06-08f53d6e4a6e-slim/test/addons/worker-addon-exit/build/Release/binding.node]
 5: 0xff2260822390 MyObject::~MyObject() [/home/runner/work/_temp/node-v27.0.0-nightly2026-06-08f53d6e4a6e-slim/test/addons/worker-addon-exit/build/Release/binding.node]
 6: 0xab6062721898  [out/Release/node]
 7: 0xab60627cfe78  [out/Release/node]
 8: 0xab60627d07b0  [out/Release/node]
 9: 0xab60627d32e4  [out/Release/node]
10: 0xab6062f7cb60  [out/Release/node]
11: 0xab60627d10cc  [out/Release/node]
12: 0xab606279ddf4  [out/Release/node]
13: 0xab606279e22c  [out/Release/node]
14: 0xab606276f160  [out/Release/node]
15: 0xab606314fbb8  [out/Release/node]
16: 0xab6064cc2c8c  [out/Release/node]
Command: out/Release/node /home/runner/work/_temp/node-v27.0.0-nightly2026-06-08f53d6e4a6e-slim/test/addons/worker-addon-exit/test.js

[nikwen]

@addaleax Will this commit be backported to Node 24? #63540 (comment) mentioned that the crash also happens on Node 24.

Thank you!