diff --git a/node.gyp b/node.gyp index 6b70bc836394ab..d37be3141bd317 100644 --- a/node.gyp +++ b/node.gyp @@ -564,7 +564,6 @@ 'src/node_zlib.cc', 'src/pipe_wrap.cc', 'src/process_wrap.cc', - 'src/sharedarraybuffer_metadata.cc', 'src/signal_wrap.cc', 'src/spawn_sync.cc', 'src/stream_base.cc', @@ -642,7 +641,6 @@ 'src/pipe_wrap.h', 'src/req_wrap.h', 'src/req_wrap-inl.h', - 'src/sharedarraybuffer_metadata.h', 'src/spawn_sync.h', 'src/stream_base.h', 'src/stream_base-inl.h', diff --git a/src/api/environment.cc b/src/api/environment.cc index 846e4a873da51c..e7d94b7fa6d9a7 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -279,6 +279,14 @@ Isolate* NewIsolate(ArrayBufferAllocator* allocator, return NewIsolate(¶ms, event_loop, platform); } +Isolate* NewIsolate(std::shared_ptr allocator, + uv_loop_t* event_loop, + MultiIsolatePlatform* platform) { + Isolate::CreateParams params; + if (allocator) params.array_buffer_allocator_shared = allocator; + return NewIsolate(¶ms, event_loop, platform); +} + IsolateData* CreateIsolateData(Isolate* isolate, uv_loop_t* loop, MultiIsolatePlatform* platform, diff --git a/src/env.cc b/src/env.cc index dd326adf2aa247..93e19493f48682 100644 --- a/src/env.cc +++ b/src/env.cc @@ -1037,21 +1037,6 @@ 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); -} - bool Environment::RunWeakRefCleanup() { isolate()->ClearKeptObjects(); diff --git a/src/env.h b/src/env.h index d6dd50b70110c5..b28ff0c0d79a7e 100644 --- a/src/env.h +++ b/src/env.h @@ -156,7 +156,6 @@ constexpr size_t kFsStatsBufferLength = V(contextify_global_private_symbol, "node:contextify:global") \ V(decorated_private_symbol, "node:decorated") \ V(napi_wrapper, "node:napi:wrapper") \ - V(sab_lifetimepartner_symbol, "node:sharedArrayBufferLifetimePartner") \ // Symbols are per-isolate primitives but Environment proxies them // for the sake of convenience. @@ -1244,10 +1243,6 @@ 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,10 +1419,6 @@ 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/memory_tracker-inl.h b/src/memory_tracker-inl.h index da37f72c737607..14635aaf5e84c8 100644 --- a/src/memory_tracker-inl.h +++ b/src/memory_tracker-inl.h @@ -109,6 +109,16 @@ void MemoryTracker::TrackField(const char* edge_name, TrackField(edge_name, value.get(), node_name); } +template +void MemoryTracker::TrackField(const char* edge_name, + const std::shared_ptr& value, + const char* node_name) { + if (value.get() == nullptr) { + return; + } + TrackField(edge_name, value.get(), node_name); +} + template void MemoryTracker::TrackField(const char* edge_name, const T& value, @@ -206,6 +216,12 @@ void MemoryTracker::TrackField(const char* edge_name, TrackFieldWithSize(edge_name, value.size, "MallocedBuffer"); } +void MemoryTracker::TrackField(const char* edge_name, + const v8::BackingStore* value, + const char* node_name) { + TrackFieldWithSize(edge_name, value->ByteLength(), "BackingStore"); +} + void MemoryTracker::TrackField(const char* name, const uv_buf_t& value, const char* node_name) { diff --git a/src/memory_tracker.h b/src/memory_tracker.h index d22116918afec8..3bcbe97c99c4e2 100644 --- a/src/memory_tracker.h +++ b/src/memory_tracker.h @@ -138,6 +138,10 @@ class MemoryTracker { inline void TrackField(const char* edge_name, const std::unique_ptr& value, const char* node_name = nullptr); + template + inline void TrackField(const char* edge_name, + const std::shared_ptr& value, + const char* node_name = nullptr); // For containers, the elements will be graphed as grandchildren nodes // if the container is not empty. @@ -197,6 +201,9 @@ class MemoryTracker { inline void TrackField(const char* edge_name, const MallocedBuffer& value, const char* node_name = nullptr); + inline void TrackField(const char* edge_name, + const v8::BackingStore* value, + const char* node_name = nullptr); // We do not implement CleanupHookCallback as MemoryRetainer // but instead specialize the method here to avoid the cost of // virtual pointers. diff --git a/src/node.h b/src/node.h index 8215552227a2aa..e023d4a17a959b 100644 --- a/src/node.h +++ b/src/node.h @@ -328,6 +328,10 @@ NODE_EXTERN v8::Isolate* NewIsolate(ArrayBufferAllocator* allocator, NODE_EXTERN v8::Isolate* NewIsolate(ArrayBufferAllocator* allocator, struct uv_loop_s* event_loop, MultiIsolatePlatform* platform); +NODE_EXTERN v8::Isolate* NewIsolate( + std::shared_ptr allocator, + struct uv_loop_s* event_loop, + MultiIsolatePlatform* platform); // Creates a new context with Node.js-specific tweaks. NODE_EXTERN v8::Local NewContext( diff --git a/src/node_messaging.cc b/src/node_messaging.cc index 19065fdb7d1be5..c2a2063381ddf1 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -12,7 +12,7 @@ using node::contextify::ContextifyContext; using v8::Array; using v8::ArrayBuffer; -using v8::ArrayBufferCreationMode; +using v8::BackingStore; using v8::Context; using v8::EscapableHandleScope; using v8::Exception; @@ -123,10 +123,9 @@ MaybeLocal Message::Deserialize(Environment* env, std::vector> shared_array_buffers; // Attach all transferred SharedArrayBuffers to their new Isolate. for (uint32_t i = 0; i < shared_array_buffers_.size(); ++i) { - Local sab; - if (!shared_array_buffers_[i]->GetSharedArrayBuffer(env, context) - .ToLocal(&sab)) - return MaybeLocal(); + Local sab = + SharedArrayBuffer::New(env->isolate(), + std::move(shared_array_buffers_[i])); shared_array_buffers.push_back(sab); } shared_array_buffers_.clear(); @@ -141,30 +140,12 @@ MaybeLocal Message::Deserialize(Environment* env, delegate.deserializer = &deserializer; // Attach all transferred ArrayBuffers to their new Isolate. - for (uint32_t i = 0; i < array_buffer_contents_.size(); ++i) { - if (!env->isolate_data()->uses_node_allocator()) { - // We don't use Node's allocator on the receiving side, so we have - // to create the ArrayBuffer from a copy of the memory. - AllocatedBuffer buf = - env->AllocateManaged(array_buffer_contents_[i].size); - memcpy(buf.data(), - array_buffer_contents_[i].data, - array_buffer_contents_[i].size); - deserializer.TransferArrayBuffer(i, buf.ToArrayBuffer()); - continue; - } - - env->isolate_data()->node_allocator()->RegisterPointer( - array_buffer_contents_[i].data, array_buffer_contents_[i].size); - + for (uint32_t i = 0; i < array_buffers_.size(); ++i) { Local ab = - ArrayBuffer::New(env->isolate(), - array_buffer_contents_[i].release(), - array_buffer_contents_[i].size, - ArrayBufferCreationMode::kInternalized); + ArrayBuffer::New(env->isolate(), std::move(array_buffers_[i])); deserializer.TransferArrayBuffer(i, ab); } - array_buffer_contents_.clear(); + array_buffers_.clear(); if (deserializer.ReadHeader(context).IsNothing()) return MaybeLocal(); @@ -173,8 +154,8 @@ MaybeLocal Message::Deserialize(Environment* env, } void Message::AddSharedArrayBuffer( - const SharedArrayBufferMetadataReference& reference) { - shared_array_buffers_.push_back(reference); + std::shared_ptr backing_store) { + shared_array_buffers_.emplace_back(std::move(backing_store)); } void Message::AddMessagePort(std::unique_ptr&& data) { @@ -249,16 +230,9 @@ class SerializerDelegate : public ValueSerializer::Delegate { } } - auto reference = SharedArrayBufferMetadata::ForSharedArrayBuffer( - env_, - context_, - shared_array_buffer); - if (!reference) { - return Nothing(); - } seen_shared_array_buffers_.emplace_back( Global { isolate, shared_array_buffer }); - msg_->AddSharedArrayBuffer(reference); + msg_->AddSharedArrayBuffer(shared_array_buffer->GetBackingStore()); return Just(i); } @@ -386,18 +360,12 @@ Maybe Message::Serialize(Environment* env, } for (Local ab : array_buffers) { - // If serialization succeeded, we want to take ownership of - // (a.k.a. externalize) the underlying memory region and render - // it inaccessible in this Isolate. - ArrayBuffer::Contents contents = ab->Externalize(); + // If serialization succeeded, we render it inaccessible in this Isolate. + std::shared_ptr backing_store = ab->GetBackingStore(); + ab->Externalize(backing_store); ab->Detach(); - CHECK(env->isolate_data()->uses_node_allocator()); - env->isolate_data()->node_allocator()->UnregisterPointer( - contents.Data(), contents.ByteLength()); - - array_buffer_contents_.emplace_back(MallocedBuffer{ - static_cast(contents.Data()), contents.ByteLength()}); + array_buffers_.emplace_back(std::move(backing_store)); } delegate.Finish(); @@ -411,9 +379,8 @@ Maybe Message::Serialize(Environment* env, } void Message::MemoryInfo(MemoryTracker* tracker) const { - tracker->TrackField("array_buffer_contents", array_buffer_contents_); - tracker->TrackFieldWithSize("shared_array_buffers", - shared_array_buffers_.size() * sizeof(shared_array_buffers_[0])); + tracker->TrackField("array_buffers_", array_buffers_); + tracker->TrackField("shared_array_buffers", shared_array_buffers_); tracker->TrackField("message_ports", message_ports_); } diff --git a/src/node_messaging.h b/src/node_messaging.h index 054521b0563c42..32eedfb34f917b 100644 --- a/src/node_messaging.h +++ b/src/node_messaging.h @@ -5,7 +5,6 @@ #include "env.h" #include "node_mutex.h" -#include "sharedarraybuffer_metadata.h" #include namespace node { @@ -52,7 +51,7 @@ class Message : public MemoryRetainer { // Internal method of Message that is called when a new SharedArrayBuffer // object is encountered in the incoming value's structure. - void AddSharedArrayBuffer(const SharedArrayBufferMetadataReference& ref); + void AddSharedArrayBuffer(std::shared_ptr backing_store); // Internal method of Message that is called once serialization finishes // and that transfers ownership of `data` to this message. void AddMessagePort(std::unique_ptr&& data); @@ -74,8 +73,8 @@ class Message : public MemoryRetainer { private: MallocedBuffer main_message_buf_; - std::vector> array_buffer_contents_; - std::vector shared_array_buffers_; + std::vector> array_buffers_; + std::vector> shared_array_buffers_; std::vector> message_ports_; std::vector wasm_modules_; diff --git a/src/node_worker.cc b/src/node_worker.cc index c18af7c055e551..a9220323225ccb 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -62,7 +62,6 @@ Worker::Worker(Environment* env, per_isolate_opts_(per_isolate_opts), exec_argv_(exec_argv), platform_(env->isolate_data()->platform()), - array_buffer_allocator_(ArrayBufferAllocator::Create()), start_profiler_idle_notifier_(env->profiler_idle_notifier_started()), thread_id_(Environment::AllocateThreadId()), env_vars_(env->env_vars()) { @@ -106,10 +105,6 @@ bool Worker::is_stopped() const { return stopped_; } -std::shared_ptr Worker::array_buffer_allocator() { - return array_buffer_allocator_; -} - void Worker::UpdateResourceConstraints(ResourceConstraints* constraints) { constraints->set_stack_limit(reinterpret_cast(stack_base_)); @@ -149,9 +144,11 @@ class WorkerThreadData { : w_(w) { CHECK_EQ(uv_loop_init(&loop_), 0); + std::shared_ptr allocator = + ArrayBufferAllocator::Create(); Isolate::CreateParams params; SetIsolateCreateParamsForNode(¶ms); - params.array_buffer_allocator = w->array_buffer_allocator_.get(); + params.array_buffer_allocator_shared = allocator; w->UpdateResourceConstraints(¶ms.constraints); @@ -175,7 +172,7 @@ class WorkerThreadData { isolate_data_.reset(CreateIsolateData(isolate, &loop_, w_->platform_, - w->array_buffer_allocator_.get())); + allocator.get())); CHECK(isolate_data_); if (w_->per_isolate_opts_) isolate_data_->set_options(std::move(w_->per_isolate_opts_)); diff --git a/src/node_worker.h b/src/node_worker.h index 46eab70a499c37..7b1311734a2a4a 100644 --- a/src/node_worker.h +++ b/src/node_worker.h @@ -48,7 +48,6 @@ class Worker : public AsyncWrap { SET_SELF_SIZE(Worker) bool is_stopped() const; - std::shared_ptr array_buffer_allocator(); static void New(const v8::FunctionCallbackInfo& args); static void CloneParentEnvVars( @@ -72,7 +71,6 @@ class Worker : public AsyncWrap { std::vector argv_; MultiIsolatePlatform* platform_; - std::shared_ptr array_buffer_allocator_; v8::Isolate* isolate_ = nullptr; bool start_profiler_idle_notifier_; uv_thread_t tid_; diff --git a/src/sharedarraybuffer_metadata.cc b/src/sharedarraybuffer_metadata.cc deleted file mode 100644 index fc3bcdf3d3b6b7..00000000000000 --- a/src/sharedarraybuffer_metadata.cc +++ /dev/null @@ -1,175 +0,0 @@ -#include "sharedarraybuffer_metadata.h" - -#include "base_object-inl.h" -#include "memory_tracker-inl.h" -#include "node_errors.h" -#include "node_worker.h" -#include "util-inl.h" - -#include - -using v8::Context; -using v8::Function; -using v8::FunctionTemplate; -using v8::Local; -using v8::Maybe; -using v8::MaybeLocal; -using v8::Nothing; -using v8::Object; -using v8::SharedArrayBuffer; -using v8::Value; - -namespace node { -namespace worker { - -namespace { - -// Yield a JS constructor for SABLifetimePartner objects in the form of a -// standard API object, that has a single field for containing the raw -// SABLifetimePartner* pointer. -Local GetSABLifetimePartnerConstructor( - Environment* env, Local context) { - Local templ; - templ = env->sab_lifetimepartner_constructor_template(); - if (!templ.IsEmpty()) - return templ->GetFunction(context).ToLocalChecked(); - - templ = BaseObject::MakeLazilyInitializedJSTemplate(env); - templ->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), - "SABLifetimePartner")); - env->set_sab_lifetimepartner_constructor_template(templ); - - return GetSABLifetimePartnerConstructor(env, context); -} - -class SABLifetimePartner : public BaseObject { - public: - SABLifetimePartner(Environment* env, - Local obj, - SharedArrayBufferMetadataReference r) - : 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() - SET_MEMORY_INFO_NAME(SABLifetimePartner) - SET_SELF_SIZE(SABLifetimePartner) - - SharedArrayBufferMetadataReference reference; -}; - -} // anonymous namespace - -SharedArrayBufferMetadataReference -SharedArrayBufferMetadata::ForSharedArrayBuffer( - Environment* env, - Local context, - Local source) { - Local lifetime_partner; - - if (!source->GetPrivate(context, - env->sab_lifetimepartner_symbol()) - .ToLocal(&lifetime_partner)) { - return nullptr; - } - - if (lifetime_partner->IsObject() && - env->sab_lifetimepartner_constructor_template() - ->HasInstance(lifetime_partner)) { - CHECK(source->IsExternal()); - SABLifetimePartner* partner = - Unwrap(lifetime_partner.As()); - CHECK_NOT_NULL(partner); - return partner->reference; - } - - if (source->IsExternal()) { - // If this is an external SharedArrayBuffer but we do not see a lifetime - // partner object, it was not us who externalized it. In that case, there - // is no way to serialize it, because it's unclear how the memory - // is actually owned. - THROW_ERR_TRANSFERRING_EXTERNALIZED_SHAREDARRAYBUFFER(env); - return nullptr; - } - - // If the SharedArrayBuffer is coming from a Worker, we need to make sure - // that the corresponding ArrayBuffer::Allocator lives at least as long as - // the SharedArrayBuffer itself. - worker::Worker* w = env->worker_context(); - std::shared_ptr allocator = - w != nullptr ? w->array_buffer_allocator() : nullptr; - - SharedArrayBuffer::Contents contents = source->Externalize(); - SharedArrayBufferMetadataReference r( - new SharedArrayBufferMetadata(contents, allocator)); - if (r->AssignToSharedArrayBuffer(env, context, source).IsNothing()) - return nullptr; - return r; -} - -Maybe SharedArrayBufferMetadata::AssignToSharedArrayBuffer( - Environment* env, Local context, - Local target) { - CHECK(target->IsExternal()); - Local ctor = GetSABLifetimePartnerConstructor(env, context); - Local obj; - if (!ctor->NewInstance(context).ToLocal(&obj)) - return Nothing(); - - new SABLifetimePartner(env, obj, shared_from_this()); - return target->SetPrivate(context, - env->sab_lifetimepartner_symbol(), - obj); -} - -SharedArrayBufferMetadata::SharedArrayBufferMetadata( - const SharedArrayBuffer::Contents& contents, - std::shared_ptr allocator) - : contents_(contents), allocator_(allocator) { } - -SharedArrayBufferMetadata::~SharedArrayBufferMetadata() { - contents_.Deleter()(contents_.Data(), - contents_.ByteLength(), - contents_.DeleterData()); -} - -MaybeLocal SharedArrayBufferMetadata::GetSharedArrayBuffer( - Environment* env, Local context) { - Local obj = - SharedArrayBuffer::New(env->isolate(), - contents_.Data(), - contents_.ByteLength()); - - if (AssignToSharedArrayBuffer(env, context, obj).IsNothing()) - return MaybeLocal(); - - return obj; -} - -} // namespace worker -} // namespace node diff --git a/src/sharedarraybuffer_metadata.h b/src/sharedarraybuffer_metadata.h deleted file mode 100644 index 4d89f08ee10daf..00000000000000 --- a/src/sharedarraybuffer_metadata.h +++ /dev/null @@ -1,72 +0,0 @@ -#ifndef SRC_SHAREDARRAYBUFFER_METADATA_H_ -#define SRC_SHAREDARRAYBUFFER_METADATA_H_ - -#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS - -#include "node.h" -#include - -namespace node { -namespace worker { - -class SharedArrayBufferMetadata; - -// This is an object associated with a SharedArrayBuffer, which keeps track -// of a cross-thread reference count. Once a SharedArrayBuffer is transferred -// for the first time (or is attempted to be transferred), one of these objects -// is created, and the SharedArrayBuffer is moved from internalized mode into -// externalized mode (i.e. the JS engine no longer frees the memory on its own). -// -// This will always be referred to using a std::shared_ptr, since it keeps -// a reference count and is guaranteed to be thread-safe. -typedef std::shared_ptr - SharedArrayBufferMetadataReference; - -class SharedArrayBufferMetadata - : public std::enable_shared_from_this { - public: - static SharedArrayBufferMetadataReference ForSharedArrayBuffer( - Environment* env, - v8::Local context, - v8::Local source); - ~SharedArrayBufferMetadata(); - - // Create a SharedArrayBuffer object for a specific Environment and Context. - // The created SharedArrayBuffer will be in externalized mode and has - // a hidden object attached to it, during whose lifetime the reference - // 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=( - SharedArrayBufferMetadata&& other) = delete; - SharedArrayBufferMetadata& operator=( - const SharedArrayBufferMetadata&) = delete; - SharedArrayBufferMetadata(const SharedArrayBufferMetadata&) = delete; - - private: - SharedArrayBufferMetadata( - const v8::SharedArrayBuffer::Contents&, - std::shared_ptr); - - // Attach a lifetime tracker object with a reference count to `target`. - v8::Maybe AssignToSharedArrayBuffer( - Environment* env, - v8::Local context, - v8::Local target); - - v8::SharedArrayBuffer::Contents contents_; - std::shared_ptr allocator_; -}; - -} // namespace worker -} // namespace node - -#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS - - -#endif // SRC_SHAREDARRAYBUFFER_METADATA_H_