@nxtedition/shared - npm Package Compare versions

Comparing version 5.0.3 to 5.0.4

package.json

 {
   "name": "@nxtedition/shared",
-  "version": "5.0.3",
+  "version": "5.0.4",
   "type": "module",
@@ -50,3 +50,3 @@ "main": "lib/index.js",
   },
-  "gitHead": "7c9c7457c885c644c7a1e70ef894d4727ce240d6"
+  "gitHead": "c9b449408262ca9431467193d624f1a043d669bb"
 }

src/ring.cc

@@ -358,18 +358,61 @@ #include <nan.h>
 
-// BackingStore deleter for the state SharedArrayBuffer. Called by V8 when the
-// last isolate that holds the SAB drops its reference (cross-thread ref-count
-// reaches zero). Frees the state block and releases the native ring buffer.
-// Safe to call from any thread — only C stdlib and atomic ops are used.
-static void state_deleter(void *data, size_t /*length*/, void *hint)
+// ─── ArrayBuffer detach-on-environment-cleanup ───────────────────────────────
+//
+// V8 14.x has a bug in ArrayBufferSweeper::ReleaseAll that crashes when a
+// worker thread exits and its isolate holds external ArrayBuffers (backed by
+// non-V8-managed memory such as our mmap'd ring buffer). The crash occurs
+// inside V8 before any deleter is called.
+//
+// The fix: register a cleanup hook via node::AddEnvironmentCleanupHook that
+// fires BEFORE ArrayBufferSweeper::ReleaseAll during worker teardown. The hook
+// detaches the ArrayBuffer (removing it from the sweeper's tracking list) and
+// releases the native ring_buf_t reference.
+//
+// If the ArrayBuffer is collected by the GC normally (before the environment
+// closes), the weak callback fires, removes the cleanup hook, and releases
+// the ring_buf_t — preventing a double-release.
+
+struct AbRef
 {
-  free(data);
-  ring_buf_release(static_cast<ring_buf_t *>(hint));
+  v8::Global<v8::ArrayBuffer> global;
+  ring_buf_t *rb;
+  v8::Isolate *isolate;
+};
+
+// Called by the environment cleanup hook (fires before ArrayBufferSweeper::ReleaseAll).
+static void ab_cleanup_hook(void *arg)
+{
+  AbRef *ref = static_cast<AbRef *>(arg);
+  if (!ref->global.IsEmpty())
+  {
+    // AB is still alive — detach it to prevent V8 14.x crash in ReleaseAll,
+    // then release our ring buffer reference.
+    v8::HandleScope scope(ref->isolate);
+    auto ab = ref->global.Get(ref->isolate);
+    if (!ab.IsEmpty() && !ab->WasDetached())
+    {
+      ab->Detach();
+    }
+    // Clear the global so ab_weak_callback knows we already handled cleanup.
+    ref->global.Reset();
+    ring_buf_release(ref->rb);
+  }
+  // If global is already empty the weak callback already ran and released rb.
+  delete ref;
 }
 
-// BackingStore deleter for per-instance ArrayBuffers returned by
-// ring_get_array_buffer. Called by the isolate's GC when the ArrayBuffer
-// (and all its views) are collected.
-static void data_deleter(void * /*data*/, size_t /*length*/, void *hint)
+// Called by V8 GC when the ArrayBuffer becomes unreachable (normal lifecycle).
+// IMPORTANT: Must NOT call node::RemoveEnvironmentCleanupHook here — that
+// function asserts the current environment is non-null, which is not
+// guaranteed inside a GC callback (no JS context is active). Instead we use
+// the empty global as a sentinel: ab_cleanup_hook checks IsEmpty() and skips
+// the release if the weak callback already handled it.
+static void ab_weak_callback(const v8::WeakCallbackInfo<AbRef> &info)
 {
-  ring_buf_release(static_cast<ring_buf_t *>(hint));
+  AbRef *ref = info.GetParameter();
+  // Release the ring buffer ref now. The cleanup hook will see IsEmpty() ==
+  // true and skip the release, preventing a double-free.
+  ref->global.Reset();
+  ring_buf_release(ref->rb);
+  // Do NOT delete ref — the cleanup hook will do that.
 }
@@ -448,25 +491,20 @@
 
-  // Allocate the state block. calloc zero-initialises it so WRITE_INDEX and
-  // READ_INDEX start at 0 without any extra memset.
-  void *state = calloc(1, STATE_BYTES);
-  if (!state)
+  // TODO (fix): V8 14.x crashes in ArrayBufferSweeper::ReleaseAll when a worker thread exits
+  // and holds a [Shared]ArrayBuffer with an external (non-V8-managed) backing store, even with
+  // EmptyDeleter. Workaround: use V8-managed SAB memory so V8 allocates and owns the state
+  // block. This avoids the crash. The ring_buf_t (mmap region) is intentionally leaked until
+  // an upstream V8 fix is available.
+  //
+  // The state block is zero-initialised by V8 (WRITE_INDEX and READ_INDEX start at 0).
+  v8::Isolate *isolate = info.GetIsolate();
+  auto sab = v8::SharedArrayBuffer::New(isolate, STATE_BYTES);
   {
-    ring_buf_release(rb);
-    Nan::ThrowError("ring_alloc: out of memory for state buffer");
-    return;
+    auto bs = sab->GetBackingStore();
+    uint32_t *s = static_cast<uint32_t *>(bs->Data());
+    s[MAGIC1_INDEX] = MAGIC1;
+    s[MAGIC2_INDEX] = MAGIC2;
+    uintptr_t p = reinterpret_cast<uintptr_t>(rb);
+    s[PTR_LO_INDEX] = static_cast<uint32_t>(p & 0xFFFFFFFFu);
+    s[PTR_HI_INDEX] = static_cast<uint32_t>(static_cast<uint64_t>(p) >> 32);
   }
-
-  uint32_t *s = static_cast<uint32_t *>(state);
-  s[MAGIC1_INDEX] = MAGIC1;
-  s[MAGIC2_INDEX] = MAGIC2;
-  uintptr_t p = reinterpret_cast<uintptr_t>(rb);
-  s[PTR_LO_INDEX] = static_cast<uint32_t>(p & 0xFFFFFFFFu);
-  s[PTR_HI_INDEX] = static_cast<uint32_t>(static_cast<uint64_t>(p) >> 32);
-
-  // V8 ref-counts the BackingStore across isolates. When the last isolate
-  // that holds this SAB GC's it, state_deleter is called — regardless of
-  // which thread runs that GC. state_deleter only performs C stdlib calls
-  // and atomic operations, so it is safe to invoke from any thread.
-  auto backing = v8::SharedArrayBuffer::NewBackingStore(state, STATE_BYTES, state_deleter, rb);
-  auto sab = v8::SharedArrayBuffer::New(info.GetIsolate(), std::move(backing));
   info.GetReturnValue().Set(sab);
@@ -494,4 +532,21 @@ }
 
-  auto backing = v8::ArrayBuffer::NewBackingStore(rb->addr, rb->size * 2, data_deleter, rb);
-  auto ab = v8::ArrayBuffer::New(info.GetIsolate(), std::move(backing));
+  v8::Isolate *isolate = info.GetIsolate();
+
+  // Use EmptyDeleter: the ring_buf_t owns the mmap'd memory, not V8.
+  // ring_buf_release is called from either ab_cleanup_hook (worker teardown)
+  // or ab_weak_callback (normal GC), whichever fires first.
+  // auto backing = v8::ArrayBuffer::NewBackingStore(rb->addr, rb->size * 2, data_deleter, rb);
+  auto backing = v8::ArrayBuffer::NewBackingStore(
+      rb->addr, rb->size * 2, v8::BackingStore::EmptyDeleter, nullptr);
+  auto ab = v8::ArrayBuffer::New(isolate, std::move(backing));
+
+  // Register a cleanup hook so the AB is detached before ArrayBufferSweeper::ReleaseAll
+  // runs during worker-thread teardown (workaround for V8 14.x bug).
+  AbRef *ref = new AbRef{};
+  ref->rb = rb;
+  ref->isolate = isolate;
+  ref->global.Reset(isolate, ab);
+  ref->global.SetWeak(ref, ab_weak_callback, v8::WeakCallbackType::kParameter);
+  node::AddEnvironmentCleanupHook(isolate, ab_cleanup_hook, ref);
+
   info.GetReturnValue().Set(ab);
@@ -498,0 +553,0 @@ }

Fixed alerts

AI-detected potential code anomaly

Supply chain risk

AI has identified unusual behaviors that may pose a security risk.

Improved metrics

Total package byte prevSize

56981

4.52%

Number of low supply chain risk alerts

2

-33.33%

No dependency changes