worker: keep allocators for transferred SAB instances alive longer

Keep the `ArrayBuffer::Allocator` behind a `SharedArrayBuffer` instance
alive for at least as long as the receiving Isolate lives, if the
`SharedArrayBuffer` instance isn’t already destroyed through GC.

This is to work around the fact that V8 7.9 started refactoring
how backing stores for `SharedArrayBuffer` instances work, changing
the timing of the call that releases the backing store to be
during Isolate disposal.

The flag added to the test is optional but helps verify that the
backing store is actually free’d at the end of the test and does not
leak memory.

Fixes: https://github.com/nodejs/node-v8/issues/115
PR-URL: https://github.com/nodejs/node/pull/29637
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This commit is contained in:
Anna Henningsen authored and Rich Trott committed 2019-09-24 21:35:40 -07:00
1 parent 355f2ad466
commit 7899a96e66
5 files changed
+51

No files matched your search

+24
View File
@@ -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()