diff --git a/src/env.cc b/src/env.cc index 257bf78519f32d..2400785ea82fe4 100644 --- a/src/env.cc +++ b/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 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(data); + }, static_cast(keep_alive_allocators_)); + } + + keep_alive_allocators_->insert(allocator); +} + void AsyncRequest::Install(Environment* env, void* data, uv_async_cb target) { CHECK_NULL(async_); env_ = env; diff --git a/src/env.h b/src/env.h index 74d65f4fbb7b8a..bdfa85fac4a8cb 100644 --- a/src/env.h +++ b/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); + private: template 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> + ArrayBufferAllocatorList; + ArrayBufferAllocatorList* keep_alive_allocators_ = nullptr; + template void ForEachBaseObject(T&& iterator); diff --git a/src/sharedarraybuffer_metadata.cc b/src/sharedarraybuffer_metadata.cc index b9d86d05ff20f3..fc3bcdf3d3b6b7 100644 --- a/src/sharedarraybuffer_metadata.cc +++ b/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(this)); + } + + ~SABLifetimePartner() { + env()->RemoveCleanupHook(CleanupHook, static_cast(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(data); + self->env()->AddArrayBufferAllocatorToKeepAliveUntilIsolateDispose( + self->reference->allocator()); } SET_NO_MEMORY_INFO() diff --git a/src/sharedarraybuffer_metadata.h b/src/sharedarraybuffer_metadata.h index 8da603978a990b..4d89f08ee10daf 100644 --- a/src/sharedarraybuffer_metadata.h +++ b/src/sharedarraybuffer_metadata.h @@ -37,6 +37,9 @@ class SharedArrayBufferMetadata // count is increased by 1. v8::MaybeLocal GetSharedArrayBuffer( Environment* env, v8::Local context); + std::shared_ptr allocator() const { + return allocator_; + } SharedArrayBufferMetadata(SharedArrayBufferMetadata&& other) = delete; SharedArrayBufferMetadata& operator=( diff --git a/test/parallel/test-worker-sharedarraybuffer-from-worker-thread.js b/test/parallel/test-worker-sharedarraybuffer-from-worker-thread.js index 60e8a5d52ab5bf..56dfe2ec41ab3a 100644 --- a/test/parallel/test-worker-sharedarraybuffer-from-worker-thread.js +++ b/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');