fix(native): remove AdjustExternalMemory from destructors to fix Node 24 crash

pproenca · pproenca · commit f348140ad1e2 · 2026-01-01T15:23:53.000Z
Node.js 24's V8 has stricter validation in ArrayBufferSweeper::~ArrayBufferSweeper() during Heap::TearDown(). Calling AdjustExternalMemory() from ObjectWrap destructors races with the sweeper's teardown, causing the assertion: Check failed: static_cast<int64_t>(amount_before) >= -delta. This manifested as crashes on Node 24.x only, with all tests passing but the process crashing during shutdown. The fix removes AdjustExternalMemory() calls from VideoFrame and AudioData destructors. External memory tracking is now handled exclusively in Close(), which the WebCodecs spec mandates users must call for proper resource management. If users violate the spec and don't call Close(), external memory accounting will be slightly off, but the process won't crash. This is acceptable since spec-violating code is a user error. Ref: nodejs/node-addon-api#1153 Ref: nodejs/node#38492
diff --git a/src/audio_data.cc b/src/audio_data.cc
@@ -207,11 +207,18 @@ AudioData::AudioData(const Napi::CallbackInfo& info)
 
 AudioData::~AudioData() {
   webcodecs::counterAudioData--;
-  // Release external memory tracking if close() was never called.
-  if (!data_.empty()) {
-    Napi::MemoryManagement::AdjustExternalMemory(
-        Env(), -static_cast<int64_t>(data_.size()));
-  }
+  // Note: We intentionally DO NOT call AdjustExternalMemory here.
+  //
+  // Calling NAPI functions (including AdjustExternalMemory) from destructors
+  // during V8 shutdown is unsafe and causes crashes on Node.js 24+ due to
+  // race conditions with V8's ArrayBufferSweeper during Heap::TearDown().
+  // See: https://github.com/nodejs/node-addon-api/issues/1153
+  //
+  // The WebCodecs spec mandates that close() must be called for proper
+  // resource management. External memory tracking is handled exclusively
+  // in Close() to avoid shutdown crashes.
+  data_.clear();
+  data_.shrink_to_fit();
 }
 
 size_t AudioData::GetBytesPerSample() const {
diff --git a/src/video_frame.cc b/src/video_frame.cc
@@ -424,13 +424,19 @@ VideoFrame::VideoFrame(const Napi::CallbackInfo& info)
 
 VideoFrame::~VideoFrame() {
   webcodecs::counterVideoFrames--;
-  // If close() wasn't called, we still need to release external memory.
-  // Note: If close() was called, data_ is already empty, so this is a no-op.
-  if (!data_.empty()) {
-    // Env() is available in destructor via ObjectWrap.
-    Napi::MemoryManagement::AdjustExternalMemory(
-        Env(), -static_cast<int64_t>(data_.size()));
-  }
+  // Note: We intentionally DO NOT call AdjustExternalMemory here.
+  //
+  // Calling NAPI functions (including AdjustExternalMemory) from destructors
+  // during V8 shutdown is unsafe and causes crashes on Node.js 24+ due to
+  // race conditions with V8's ArrayBufferSweeper during Heap::TearDown().
+  // See: https://github.com/nodejs/node-addon-api/issues/1153
+  //
+  // The WebCodecs spec mandates that close() must be called for proper
+  // resource management. External memory tracking is handled exclusively
+  // in Close() to avoid shutdown crashes.
+  //
+  // If close() wasn't called, external memory accounting will be slightly off,
+  // but this is a user error (violating WebCodecs spec) and won't crash.
   data_.clear();
   data_.shrink_to_fit();
 }
