worker: fix crash when SharedArrayBuffer outlives creating thread

Use the parent thread’s `ArrayBuffer::Allocator` when creating a
Worker instance, as that allocator is guaranteed to outlive the
Worker itself.

This requires making the zero-fill flag a thread_local variable
in order to avoid race conditions between different threads.
A test for that behaviour is added as well.

Fixes: #28777
Fixes: #28773

Author: addaleax

src/api/environment.cc

@@ -76,6 +76,8 @@ static MaybeLocal<Value> PrepareStackTraceCallback(Local<Context> context,
   return result;
 }
 
+thread_local uint32_t NodeArrayBufferAllocator::zero_fill_field_ = 1;
+
 void* NodeArrayBufferAllocator::Allocate(size_t size) {
   if (zero_fill_field_ || per_process::cli_options->zero_fill_all_buffers)
     return UncheckedCalloc(size);

src/env.cc

@@ -930,6 +930,7 @@ void AsyncHooks::MemoryInfo(MemoryTracker* tracker) const {
 void AsyncHooks::grow_async_ids_stack() {
   async_ids_stack_.reserve(async_ids_stack_.Length() * 3);
 
+  CHECK(!env()->async_hooks_binding().IsEmpty());
   env()->async_hooks_binding()->Set(
       env()->context(),
       env()->async_ids_stack_string(),

src/node_internals.h

@@ -121,7 +121,8 @@ class NodeArrayBufferAllocator : public ArrayBufferAllocator {
   NodeArrayBufferAllocator* GetImpl() final { return this; }
 
  private:
-  uint32_t zero_fill_field_ = 1;  // Boolean but exposed as uint32 to JS land.
+  // Boolean but exposed as uint32 to JS land.
+  static thread_local uint32_t zero_fill_field_;
 };
 
 class DebuggingArrayBufferAllocator final : public NodeArrayBufferAllocator {

src/node_worker.cc

@@ -18,7 +18,6 @@
 
 using node::options_parser::kDisallowedInEnvironment;
 using v8::Array;
-using v8::ArrayBuffer;
 using v8::Boolean;
 using v8::Context;
 using v8::Function;
@@ -107,12 +106,13 @@ bool Worker::is_stopped() const {
 // (Eventually, the Environment instance should probably also be moved here.)
 class WorkerThreadData {
  public:
-  explicit WorkerThreadData(Worker* w)
-    : w_(w),
-      array_buffer_allocator_(ArrayBufferAllocator::Create()) {
-    CHECK_EQ(uv_loop_init(&loop_), 0);
+  explicit WorkerThreadData(Worker* w) : w_(w) {
+    IsolateData* parent_isolate_data = w->env()->isolate_data();
 
-    Isolate* isolate = NewIsolate(array_buffer_allocator_.get(), &loop_);
+    CHECK_EQ(uv_loop_init(&loop_), 0);
+    Isolate::CreateParams params;
+    params.array_buffer_allocator = parent_isolate_data->allocator();
+    Isolate* isolate = NewIsolate(&params, &loop_, w->platform_);
     CHECK_NOT_NULL(isolate);
 
     {
@@ -121,10 +121,11 @@ class WorkerThreadData {
       isolate->SetStackLimit(w_->stack_base_);
 
       HandleScope handle_scope(isolate);
-      isolate_data_.reset(CreateIsolateData(isolate,
-                                            &loop_,
-                                            w_->platform_,
-                                            array_buffer_allocator_.get()));
+      isolate_data_.reset(CreateIsolateData(
+          isolate,
+          &loop_,
+          w_->platform_,
+          parent_isolate_data->node_allocator()));
       CHECK(isolate_data_);
       if (w_->per_isolate_opts_)
         isolate_data_->set_options(std::move(w_->per_isolate_opts_));
@@ -166,7 +167,6 @@ class WorkerThreadData {
  private:
   Worker* const w_;
   uv_loop_t loop_;
-  std::unique_ptr<ArrayBufferAllocator> array_buffer_allocator_;
   DeleteFnPtr<IsolateData, FreeIsolateData> isolate_data_;
 
   friend class Worker;

test/parallel/test-worker-arraybuffer-zerofill.js

@@ -0,0 +1,33 @@
+'use strict';
+require('../common');
+const assert = require('assert');
+const { Worker } = require('worker_threads');
+
+// Make sure that allocating uninitialized ArrayBuffers in one thread does not
+// affect the zero-initialization in other threads.
+
+const w = new Worker(`
+const { parentPort } = require('worker_threads');
+
+function post() {
+  const uint32array = new Uint32Array(64);
+  parentPort.postMessage(uint32array.reduce((a, b) => a + b));
+}
+
+setInterval(post, 0);
+`, { eval: true });
+
+function allocBuffers() {
+  Buffer.allocUnsafe(32 * 1024 * 1024);
+}
+
+const interval = setInterval(allocBuffers, 0);
+
+let messages = 0;
+w.on('message', (sum) => {
+  assert.strictEqual(sum, 0);
+  if (messages++ === 100) {
+    clearInterval(interval);
+    w.terminate();
+  }
+});

test/parallel/test-worker-sharedarraybuffer-from-worker-thread.js

@@ -0,0 +1,22 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const { Worker } = require('worker_threads');
+
+// Regression test for https://github.com/nodejs/node/issues/28777
+// Make sure that SharedArrayBuffers created in Worker threads are accessible
+// after the creating thread ended.
+
+const w = new Worker(`
+const { parentPort } = require('worker_threads');
+const sharedArrayBuffer = new SharedArrayBuffer(4);
+parentPort.postMessage(sharedArrayBuffer);
+`, { eval: true });
+
+let sharedArrayBuffer;
+w.once('message', common.mustCall((message) => sharedArrayBuffer = message));
+w.once('exit', common.mustCall(() => {
+  const uint8array = new Uint8Array(sharedArrayBuffer);
+  uint8array[0] = 42;
+  assert.deepStrictEqual(uint8array, new Uint8Array([42, 0, 0, 0]));
+}));