`ObjectWrap` cleanup hooks break pre-built addons on Node.js 26.0.0-26.3.1 runtimes

[dyalevog]

Version

Node.js headers 26.4.0 or newer
Node.js runtime 26.0.0 up to and including 26.3.1

Platform

Not platform specific, but used:
Linux 7.0.0-28-generic #28~24.04.1-Ubuntu x86_64 x86_64 x86_64 GNU/Linux

Subsystem

src, addons

What steps will reproduce the bug?

The crash depends on header and runtime version. So you will need headers from e.g. Node.js 26.4.0 and two runtimes (e.g. 26.2.0 and 26.4.0)

1. Sample that just uses ObjectWrap
   addon.cc:

#include <node.h>
#include <node_object_wrap.h>

using namespace v8;

class Thing : public node::ObjectWrap {
 public:
  static void New(const FunctionCallbackInfo<Value>& args) {
    (new Thing())->Wrap(args.This());
  }
};

void Init(Local<Object> exports) {
  Isolate* isolate = Isolate::GetCurrent();
  Local<Context> context = isolate->GetCurrentContext();
  Local<FunctionTemplate> tpl = FunctionTemplate::New(isolate, Thing::New);
  tpl->InstanceTemplate()->SetInternalFieldCount(1);
  exports->Set(context,
               String::NewFromUtf8(isolate, "Thing").ToLocalChecked(),
               tpl->GetFunction(context).ToLocalChecked()).Check();
}

NODE_MODULE(NODE_GYP_MODULE_NAME, Init)

index.js:

const { Thing } = require('./addon.node');

let junk = [];
for (let i = 0; i < 300000; i++) {
  new Thing();                          // wrapped object, immediately unreachable
  junk.push({ a: i });                  // keep allocation rate high so V8 collects on its own
  if (junk.length > 1000) junk = [];
}
console.log('survived');

2. Compile against 26.4.0 (or newer if you want)

g++ -std=c++20 -fPIC -shared \
    -I</path/to/node-v26.4.0-linux-x64/include/node> \
    -DNODE_GYP_MODULE_NAME=addon \
    -o addon.node addon.cc

3. Run same binary against two different Node.js versions

# this one will run fine
<path/to/node_26.4> index.js

# this one will crash
<path/to/node_26.2> index.js

How often does it reproduce? Is there a required condition?

Deterministically with 5/5 runs.
See this table for when it crashes:

hdr \ rt	26.0.0	26.3.1	26.4.0	26.7.0
26.0.0	ok	ok	ok	ok
26.3.1	ok	ok	ok	ok
26.4.0	crash	crash	ok	ok
26.7.0	crash	crash	ok	ok

I.e. it crashes with headers >= 26.4.0 when runtime <= 26.3.1.

A explicit trigger of the gc via global.gc() will not cause the failure, it has to be an allocation-driven one.

What is the expected behavior? Why is that the expected behavior?

A pre-built addon that is built against headers of one 26.x release should keep working on another 26.x runtime

What do you see instead?

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

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

 1: 0x813228 node::Assert(node::AssertionInfo const&) [node]
 2: 0x891b90 node::RemoveEnvironmentCleanupHook(v8::Isolate*, void (*)(void*), void*) [node]
 3: 0x73f036ca4045 node::ObjectWrap::RemoveCleanupHook() [/home/user/reproducer/addon.node]
 4: 0x73f036ca393a node::ObjectWrap::~ObjectWrap() [/home/user/reproducer/addon.node]
 5: 0x73f036ca41ec Thing::~Thing() [/home/user/reproducer/addon.node]
 6: 0x73f036ca420c Thing::~Thing() [/home/user/reproducer/addon.node]
 7: 0x73f036ca3fdd node::ObjectWrap::WeakCallback(v8::WeakCallbackInfo<node::ObjectWrap> const&) [/home/user/reproducer/addon.node]
 8: 0xe18a09  [node]
 9: 0xeb3f61  [node]
10: 0xec4582  [node]
11: 0xec425f  [node]
12: 0x194cdf7  [node]
13: 0xeb0372  [node]
14: 0xea3914  [node]
15: 0xea373c  [node]
16: 0xea30cf  [node]
17: 0xe7f25d  [node]
18: 0x133d781  [node]
19: 0x73f02763ea76 
Aborted (core dumped)

Additional information

The causing code change in 26.4.0 is the RemoveCleanupHook(); line added to destructor in commit 7ac3fe1. The call interacts with the improvement of the RemoveEnvironmentCleanupHook function in 1723773.

When the destructor is called due to headers being 26.4.0 or newer, but the runtime being <= 26.3.1, then it can trigger the CHECK_NOT_NULL(env); of the RemoveEnvironmentCleanupHook function.

Refs: #63642 #63985