diff options
| -rw-r--r-- | src/env.cc | 15 | ||||
| -rw-r--r-- | src/env.h | 8 | ||||
| -rw-r--r-- | src/sharedarraybuffer_metadata.cc | 24 | ||||
| -rw-r--r-- | src/sharedarraybuffer_metadata.h | 3 | ||||
| -rw-r--r-- | test/parallel/test-worker-sharedarraybuffer-from-worker-thread.js | 1 |
5 files changed, 51 insertions, 0 deletions
diff --git a/src/env.cc b/src/env.cc index 257bf78519..2400785ea8 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<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; @@ -1251,6 +1251,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, @@ -1425,6 +1429,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); diff --git a/src/sharedarraybuffer_metadata.cc b/src/sharedarraybuffer_metadata.cc index b9d86d05ff..fc3bcdf3d3 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<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() diff --git a/src/sharedarraybuffer_metadata.h b/src/sharedarraybuffer_metadata.h index 8da603978a..4d89f08ee1 100644 --- a/src/sharedarraybuffer_metadata.h +++ b/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=( diff --git a/test/parallel/test-worker-sharedarraybuffer-from-worker-thread.js b/test/parallel/test-worker-sharedarraybuffer-from-worker-thread.js index 60e8a5d52a..56dfe2ec41 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'); |