worker: keep allocators for transferred SAB instances alive longer

src/env.cc

@@ -1036,6 +1036,21 @@ char* Environment::Reallocate(char* data, size_t old_size, size_t size) {
  return new_data;
 }
 
+void Environment::AddArrayBufferAllocatorToKeepAliveUntilIsolateDispose(
+ std::shared_ptr<v8::ArrayBuffer::Allocator> allocator) {
+ if (keep_alive_allocators_ == nullptr) {
+ MultiIsolatePlatform* platform = isolate_data()->platform();
+ CHECK_NOT_NULL(platform);
+
+ keep_alive_allocators_ = new ArrayBufferAllocatorList();
+ platform->AddIsolateFinishedCallback(isolate(), [](void* data) {
+ delete static_cast<ArrayBufferAllocatorList*>(data);
+ }, static_cast<void*>(keep_alive_allocators_));
+ }
+
+ keep_alive_allocators_->insert(allocator);
+}
+
 void AsyncRequest::Install(Environment* env, void* data, uv_async_cb target) {
  CHECK_NULL(async_);
  env_ = env;

src/env.h

@@ -1250,6 +1250,10 @@ class Environment : public MemoryRetainer {
 
 #endif // HAVE_INSPECTOR
 
+ // Only available if a MultiIsolatePlatform is in use.
+ void AddArrayBufferAllocatorToKeepAliveUntilIsolateDispose(
+ std::shared_ptr<v8::ArrayBuffer::Allocator>);
+
  private:
  template <typename Fn>
  inline void CreateImmediate(Fn&& cb,
@@ -1424,6 +1428,10 @@ class Environment : public MemoryRetainer {
  // Used by embedders to shutdown running Node instance.
  AsyncRequest thread_stopper_;
 
+ typedef std::unordered_set<std::shared_ptr<v8::ArrayBuffer::Allocator>>
+ ArrayBufferAllocatorList;
+ ArrayBufferAllocatorList* keep_alive_allocators_ = nullptr;
+
  template <typename T>
  void ForEachBaseObject(T&& iterator);
 

src/sharedarraybuffer_metadata.cc

@@ -50,6 +50,30 @@ class SABLifetimePartner : public BaseObject {
  : BaseObject(env, obj),
  reference(std::move(r)) {
  MakeWeak();
+ env->AddCleanupHook(CleanupHook, static_cast<void*>(this));
+ }
+
+ ~SABLifetimePartner() {
+ env()->RemoveCleanupHook(CleanupHook, static_cast<void*>(this));
+ }
+
+ static void CleanupHook(void* data) {
+ // There is another cleanup hook attached to this object because it is a
+ // BaseObject. Cleanup hooks are triggered in reverse order of addition,
+ // so if this object is destroyed through GC, the destructor removes all
+ // hooks associated with this object, meaning that this cleanup hook
+ // only runs at the end of the Environment’s lifetime.
+ // In that case, V8 still knows about the SharedArrayBuffer and tries to
+ // free it when the last Isolate with access to it is disposed; for that,
+ // the ArrayBuffer::Allocator needs to be kept alive longer than this
+ // object and longer than the Environment instance.
+ //
+ // This is a workaround for https://github.com/nodejs/node-v8/issues/115
+ // (introduced in V8 7.9) and we should be able to remove it once V8
+ // ArrayBuffer::Allocator refactoring/removal is complete.
+ SABLifetimePartner* self = static_cast<SABLifetimePartner*>(data);
+ self->env()->AddArrayBufferAllocatorToKeepAliveUntilIsolateDispose(
+ self->reference->allocator());
  }
 
  SET_NO_MEMORY_INFO()

src/sharedarraybuffer_metadata.h

@@ -37,6 +37,9 @@ class SharedArrayBufferMetadata
  // count is increased by 1.
  v8::MaybeLocal<v8::SharedArrayBuffer> GetSharedArrayBuffer(
  Environment* env, v8::Local<v8::Context> context);
+ std::shared_ptr<v8::ArrayBuffer::Allocator> allocator() const {
+ return allocator_;
+ }
 
  SharedArrayBufferMetadata(SharedArrayBufferMetadata&& other) = delete;
  SharedArrayBufferMetadata& operator=(

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

@@ -1,3 +1,4 @@
+// Flags: --debug-arraybuffer-allocations
 'use strict';
 const common = require('../common');
 const assert = require('assert');