From 8dc939aac3ae6eed8b6c977e2940621d974256a5 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 10 Aug 2022 00:04:44 +0800 Subject: [PATCH] src: support WeakReference in snapshot Move util::WeakReference to a separate header and implement {de}serialization for it to be snapshotable. PR-URL: https://github.com/nodejs/node/pull/44193 Refs: https://github.com/nodejs/node/issues/44014 Refs: https://github.com/nodejs/node/issues/37476 Reviewed-By: Matteo Collina Reviewed-By: Chengzhong Wu --- node.gyp | 1 + src/node_snapshotable.cc | 1 + src/node_snapshotable.h | 3 +- src/node_util.cc | 129 +++++++++++++----- src/node_util.h | 54 ++++++++ src/util.cc | 1 + test/fixtures/snapshot/weak-reference-gc.js | 20 +++ test/fixtures/snapshot/weak-reference.js | 15 ++ test/parallel/test-snapshot-weak-reference.js | 60 ++++++++ 9 files changed, 246 insertions(+), 38 deletions(-) create mode 100644 src/node_util.h create mode 100644 test/fixtures/snapshot/weak-reference-gc.js create mode 100644 test/fixtures/snapshot/weak-reference.js create mode 100644 test/parallel/test-snapshot-weak-reference.js diff --git a/node.gyp b/node.gyp index e3b21af8a0d407..e20acf108f5587 100644 --- a/node.gyp +++ b/node.gyp @@ -638,6 +638,7 @@ 'src/node_stat_watcher.h', 'src/node_union_bytes.h', 'src/node_url.h', + 'src/node_util.h', 'src/node_version.h', 'src/node_v8.h', 'src/node_v8_platform-inl.h', diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index b9675b0aa9bb2f..4399b7a3cc132d 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -15,6 +15,7 @@ #include "node_metadata.h" #include "node_process.h" #include "node_snapshot_builder.h" +#include "node_util.h" #include "node_v8.h" #include "node_v8_platform-inl.h" diff --git a/src/node_snapshotable.h b/src/node_snapshotable.h index f19a7e9c77f8e0..d12aec586c7e12 100644 --- a/src/node_snapshotable.h +++ b/src/node_snapshotable.h @@ -18,7 +18,8 @@ class ExternalReferenceRegistry; V(fs_binding_data, fs::BindingData) \ V(v8_binding_data, v8_utils::BindingData) \ V(blob_binding_data, BlobBindingData) \ - V(process_binding_data, process::BindingData) + V(process_binding_data, process::BindingData) \ + V(util_weak_reference, util::WeakReference) enum class EmbedderObjectType : uint8_t { #define V(PropertyName, NativeType) k_##PropertyName, diff --git a/src/node_util.cc b/src/node_util.cc index 1613a276c58111..58f58478e59944 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -1,3 +1,4 @@ +#include "node_util.h" #include "base_object-inl.h" #include "node_errors.h" #include "node_external_reference.h" @@ -15,7 +16,7 @@ using v8::Context; using v8::External; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; -using v8::Global; +using v8::HandleScope; using v8::IndexFilter; using v8::Integer; using v8::Isolate; @@ -207,52 +208,106 @@ void ArrayBufferViewHasBuffer(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(args[0].As()->HasBuffer()); } -class WeakReference : public BaseObject { - public: - WeakReference(Environment* env, Local object, Local target) - : BaseObject(env, object) { - MakeWeak(); +WeakReference::WeakReference(Environment* env, + Local object, + Local target) + : WeakReference(env, object, target, 0) {} + +WeakReference::WeakReference(Environment* env, + Local object, + Local target, + uint64_t reference_count) + : SnapshotableObject(env, object, type_int), + reference_count_(reference_count) { + MakeWeak(); + if (!target.IsEmpty()) { target_.Reset(env->isolate(), target); - target_.SetWeak(); + if (reference_count_ == 0) { + target_.SetWeak(); + } } +} - static void New(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - CHECK(args.IsConstructCall()); - CHECK(args[0]->IsObject()); - new WeakReference(env, args.This(), args[0].As()); +bool WeakReference::PrepareForSerialization(Local context, + v8::SnapshotCreator* creator) { + if (target_.IsEmpty()) { + target_index_ = 0; + return true; } - static void Get(const FunctionCallbackInfo& args) { - WeakReference* weak_ref = Unwrap(args.Holder()); - Isolate* isolate = args.GetIsolate(); - if (!weak_ref->target_.IsEmpty()) - args.GetReturnValue().Set(weak_ref->target_.Get(isolate)); - } + // Users can still hold strong references to target in addition to the + // reference that we manage here, and they could expect that the referenced + // object remains the same as long as that external strong reference + // is alive. Since we have no way to know if there is any other reference + // keeping the target alive, the best we can do to maintain consistency is to + // simply save a reference to the target in the snapshot (effectively making + // it strong) during serialization, and restore it during deserialization. + // If there's no known counted reference from our side, we'll make the + // reference here weak upon deserialization so that it can be GC'ed if users + // do not hold additional references to it. + Local target = target_.Get(context->GetIsolate()); + target_index_ = creator->AddData(context, target); + DCHECK_NE(target_index_, 0); + target_.Reset(); + return true; +} - static void IncRef(const FunctionCallbackInfo& args) { - WeakReference* weak_ref = Unwrap(args.Holder()); - weak_ref->reference_count_++; - if (weak_ref->target_.IsEmpty()) return; - if (weak_ref->reference_count_ == 1) weak_ref->target_.ClearWeak(); - } +InternalFieldInfoBase* WeakReference::Serialize(int index) { + DCHECK_EQ(index, BaseObject::kEmbedderType); + InternalFieldInfo* info = + InternalFieldInfoBase::New(type()); + info->target = target_index_; + info->reference_count = reference_count_; + return info; +} - static void DecRef(const FunctionCallbackInfo& args) { - WeakReference* weak_ref = Unwrap(args.Holder()); - CHECK_GE(weak_ref->reference_count_, 1); - weak_ref->reference_count_--; - if (weak_ref->target_.IsEmpty()) return; - if (weak_ref->reference_count_ == 0) weak_ref->target_.SetWeak(); +void WeakReference::Deserialize(Local context, + Local holder, + int index, + InternalFieldInfoBase* info) { + DCHECK_EQ(index, BaseObject::kEmbedderType); + HandleScope scope(context->GetIsolate()); + + InternalFieldInfo* weak_info = reinterpret_cast(info); + Local target; + if (weak_info->target != 0) { + target = context->GetDataFromSnapshotOnce(weak_info->target) + .ToLocalChecked(); } + new WeakReference(Environment::GetCurrent(context), + holder, + target, + weak_info->reference_count); +} + +void WeakReference::New(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + CHECK(args.IsConstructCall()); + CHECK(args[0]->IsObject()); + new WeakReference(env, args.This(), args[0].As()); +} + +void WeakReference::Get(const FunctionCallbackInfo& args) { + WeakReference* weak_ref = Unwrap(args.Holder()); + Isolate* isolate = args.GetIsolate(); + if (!weak_ref->target_.IsEmpty()) + args.GetReturnValue().Set(weak_ref->target_.Get(isolate)); +} - SET_MEMORY_INFO_NAME(WeakReference) - SET_SELF_SIZE(WeakReference) - SET_NO_MEMORY_INFO() +void WeakReference::IncRef(const FunctionCallbackInfo& args) { + WeakReference* weak_ref = Unwrap(args.Holder()); + weak_ref->reference_count_++; + if (weak_ref->target_.IsEmpty()) return; + if (weak_ref->reference_count_ == 1) weak_ref->target_.ClearWeak(); +} - private: - Global target_; - uint64_t reference_count_ = 0; -}; +void WeakReference::DecRef(const FunctionCallbackInfo& args) { + WeakReference* weak_ref = Unwrap(args.Holder()); + CHECK_GE(weak_ref->reference_count_, 1); + weak_ref->reference_count_--; + if (weak_ref->target_.IsEmpty()) return; + if (weak_ref->reference_count_ == 0) weak_ref->target_.SetWeak(); +} static void GuessHandleType(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); diff --git a/src/node_util.h b/src/node_util.h new file mode 100644 index 00000000000000..a68d551fdb481c --- /dev/null +++ b/src/node_util.h @@ -0,0 +1,54 @@ + +#ifndef SRC_NODE_UTIL_H_ +#define SRC_NODE_UTIL_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS +#include "base_object.h" +#include "node_snapshotable.h" +#include "v8.h" + +namespace node { +namespace util { + +class WeakReference : public SnapshotableObject { + public: + SERIALIZABLE_OBJECT_METHODS(); + + static constexpr FastStringKey type_name{"node::util::WeakReference"}; + static constexpr EmbedderObjectType type_int = + EmbedderObjectType::k_util_weak_reference; + + WeakReference(Environment* env, + v8::Local object, + v8::Local target); + static void New(const v8::FunctionCallbackInfo& args); + static void Get(const v8::FunctionCallbackInfo& args); + static void IncRef(const v8::FunctionCallbackInfo& args); + static void DecRef(const v8::FunctionCallbackInfo& args); + + SET_MEMORY_INFO_NAME(WeakReference) + SET_SELF_SIZE(WeakReference) + SET_NO_MEMORY_INFO() + + struct InternalFieldInfo : public node::InternalFieldInfoBase { + SnapshotIndex target; + uint64_t reference_count; + }; + + private: + WeakReference(Environment* env, + v8::Local object, + v8::Local target, + uint64_t reference_count); + v8::Global target_; + uint64_t reference_count_ = 0; + + SnapshotIndex target_index_ = 0; // 0 means target_ is not snapshotted +}; + +} // namespace util +} // namespace node + +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#endif // SRC_NODE_UTIL_H_ diff --git a/src/util.cc b/src/util.cc index 6bbd9fd926f61a..584a6a32d6cc5b 100644 --- a/src/util.cc +++ b/src/util.cc @@ -27,6 +27,7 @@ #include "node_buffer.h" #include "node_errors.h" #include "node_internals.h" +#include "node_util.h" #include "string_bytes.h" #include "uv.h" diff --git a/test/fixtures/snapshot/weak-reference-gc.js b/test/fixtures/snapshot/weak-reference-gc.js new file mode 100644 index 00000000000000..d8bfdf95d1772a --- /dev/null +++ b/test/fixtures/snapshot/weak-reference-gc.js @@ -0,0 +1,20 @@ +'use strict'; + +const { internalBinding } = require('internal/test/binding'); +const { WeakReference } = internalBinding('util'); +const { + setDeserializeMainFunction +} = require('v8').startupSnapshot +const assert = require('assert'); + +let obj = { hello: 'world' }; +const ref = new WeakReference(obj); + +setDeserializeMainFunction(() => { + obj = null; + globalThis.gc(); + + setImmediate(() => { + assert.strictEqual(ref.get(), undefined); + }); +}); diff --git a/test/fixtures/snapshot/weak-reference.js b/test/fixtures/snapshot/weak-reference.js new file mode 100644 index 00000000000000..214d52fee185fe --- /dev/null +++ b/test/fixtures/snapshot/weak-reference.js @@ -0,0 +1,15 @@ +'use strict'; + +const { internalBinding } = require('internal/test/binding'); +const { WeakReference } = internalBinding('util'); +const { + setDeserializeMainFunction +} = require('v8').startupSnapshot +const assert = require('assert'); + +let obj = { hello: 'world' }; +const ref = new WeakReference(obj); + +setDeserializeMainFunction(() => { + assert.strictEqual(ref.get(), obj); +}); diff --git a/test/parallel/test-snapshot-weak-reference.js b/test/parallel/test-snapshot-weak-reference.js new file mode 100644 index 00000000000000..6de072290da257 --- /dev/null +++ b/test/parallel/test-snapshot-weak-reference.js @@ -0,0 +1,60 @@ +'use strict'; + +// This tests that weak references work across serialization. + +require('../common'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); +const tmpdir = require('../common/tmpdir'); +const fixtures = require('../common/fixtures'); +const path = require('path'); +const fs = require('fs'); + +tmpdir.refresh(); +const blobPath = path.join(tmpdir.path, 'snapshot.blob'); + +function runTest(entry) { + console.log('running test with', entry); + { + const child = spawnSync(process.execPath, [ + '--expose-internals', + '--expose-gc', + '--snapshot-blob', + blobPath, + '--build-snapshot', + entry, + ], { + cwd: tmpdir.path + }); + if (child.status !== 0) { + console.log(child.stderr.toString()); + console.log(child.stdout.toString()); + assert.strictEqual(child.status, 0); + } + const stats = fs.statSync(path.join(tmpdir.path, 'snapshot.blob')); + assert(stats.isFile()); + } + + { + const child = spawnSync(process.execPath, [ + '--expose-internals', + '--expose-gc', + '--snapshot-blob', + blobPath, + ], { + cwd: tmpdir.path, + env: { + ...process.env, + } + }); + + const stdout = child.stdout.toString().trim(); + const stderr = child.stderr.toString().trim(); + console.log(`[stdout]:\n${stdout}\n`); + console.log(`[stderr]:\n${stderr}\n`); + assert.strictEqual(child.status, 0); + } +} + +runTest(fixtures.path('snapshot', 'weak-reference.js')); +runTest(fixtures.path('snapshot', 'weak-reference-gc.js'));