Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: fix memory leak in v8.serialize() #37030

Merged
merged 1 commit into from
Jan 26, 2023
Merged

chore: fix memory leak in v8.serialize() #37030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion patches/node/fixup_for_wc_98-compat-extra-semi.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Wc++98-compat-extra-semi is turned on for Electron so this
patch fixes that error in node.

diff --git a/src/node_serdes.cc b/src/node_serdes.cc
index 0cd76078218433b46c17f350e3ba6073987438cf..ba13061b6aa7fd8f877aa456db9d352a847e682a 100644
index 97917c91c06dc47dfa6be2c194944cdc93e6bd7f..177390a24eb6490b128e22c104014e80f338c9d9 100644
--- a/src/node_serdes.cc
+++ b/src/node_serdes.cc
@@ -32,7 +32,7 @@ namespace serdes {
Expand Down
35 changes: 25 additions & 10 deletions patches/node/support_v8_sandboxed_pointers.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ index f27e03aed66fed5a4dc59ec3ab1102a9ea2c8c56..6d315edb9ce87fe8cce8af91bb45fd08

// Delegate to V8's allocator for compatibility with the V8 memory cage.
diff --git a/src/node_serdes.cc b/src/node_serdes.cc
index 45a16d9de43703c2115dde85c9faae3a04be2a88..0cd76078218433b46c17f350e3ba6073987438cf 100644
index 45a16d9de43703c2115dde85c9faae3a04be2a88..97917c91c06dc47dfa6be2c194944cdc93e6bd7f 100644
--- a/src/node_serdes.cc
+++ b/src/node_serdes.cc
@@ -29,6 +29,11 @@ using v8::ValueSerializer;
Expand Down Expand Up @@ -219,17 +219,32 @@ index 45a16d9de43703c2115dde85c9faae3a04be2a88..0cd76078218433b46c17f350e3ba6073
Maybe<bool> SerializerContext::WriteHostObject(Isolate* isolate,
Local<Object> input) {
MaybeLocal<Value> ret;
@@ -211,7 +240,12 @@ void SerializerContext::ReleaseBuffer(const FunctionCallbackInfo<Value>& args) {
@@ -209,9 +238,14 @@ void SerializerContext::ReleaseBuffer(const FunctionCallbackInfo<Value>& args) {
// Note: Both ValueSerializer and this Buffer::New() variant use malloc()
// as the underlying allocator.
std::pair<uint8_t*, size_t> ret = ctx->serializer_.Release();
auto buf = Buffer::New(ctx->env(),
reinterpret_cast<char*>(ret.first),
- auto buf = Buffer::New(ctx->env(),
- reinterpret_cast<char*>(ret.first),
- ret.second);
+ ret.second,
+ [](char* data, void* hint){
+ if (data)
+ GetAllocator()->Free(data, reinterpret_cast<size_t>(hint));
+ },
+ reinterpret_cast<void*>(ctx->last_length_));
+ std::unique_ptr<v8::BackingStore> bs =
+ v8::ArrayBuffer::NewBackingStore(reinterpret_cast<char*>(ret.first), ret.second,
+ [](void* data, size_t length, void* deleter_data) {
+ if (data) GetAllocator()->Free(reinterpret_cast<char*>(data), length);
+ }, nullptr);
+ Local<ArrayBuffer> ab = v8::ArrayBuffer::New(ctx->env()->isolate(), std::move(bs));
+
+ auto buf = Buffer::New(ctx->env(), ab, 0, ret.second);

if (!buf.IsEmpty()) {
args.GetReturnValue().Set(buf.ToLocalChecked());
diff --git a/test/parallel/test-v8-serialize-leak.js b/test/parallel/test-v8-serialize-leak.js
index a90c398adcdaf30491a0fecdcf00895038d62e69..f5b8a1430ad2033eae06ca0157af2fb51d3f36a5 100644
--- a/test/parallel/test-v8-serialize-leak.js
+++ b/test/parallel/test-v8-serialize-leak.js
@@ -23,5 +23,5 @@ const after = process.memoryUsage.rss();
if (process.config.variables.asan) {
assert(after < before * 10, `asan: before=${before} after=${after}`);
} else {
- assert(after < before * 2, `before=${before} after=${after}`);
+ assert(after < before * 3, `before=${before} after=${after}`);
}
1 change: 0 additions & 1 deletion script/node-disabled-tests.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@
"parallel/test-worker-debug",
"parallel/test-worker-init-failure",
"parallel/test-worker-stdio",
"parallel/test-v8-serialize-leak",
"parallel/test-zlib-unused-weak",
"report/test-report-fatalerror-oomerror-set",
"report/test-report-fatalerror-oomerror-directory",
Expand Down