From 188f7c564b827eef3748e440ee2f87363c94a800 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Thu, 26 Jan 2023 06:44:17 +0000 Subject: [PATCH] chore: fix memory leak in v8.serialize() Co-authored-by: Shelley Vohr --- .../fixup_for_wc_98-compat-extra-semi.patch | 2 +- .../node/support_v8_sandboxed_pointers.patch | 35 +++++++++++++------ script/node-disabled-tests.json | 1 - 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/patches/node/fixup_for_wc_98-compat-extra-semi.patch b/patches/node/fixup_for_wc_98-compat-extra-semi.patch index 59286b5160b9e..f3e585f58e1d4 100644 --- a/patches/node/fixup_for_wc_98-compat-extra-semi.patch +++ b/patches/node/fixup_for_wc_98-compat-extra-semi.patch @@ -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 { diff --git a/patches/node/support_v8_sandboxed_pointers.patch b/patches/node/support_v8_sandboxed_pointers.patch index feb7dbd5029dc..086cc0f6b1289 100644 --- a/patches/node/support_v8_sandboxed_pointers.patch +++ b/patches/node/support_v8_sandboxed_pointers.patch @@ -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; @@ -219,17 +219,32 @@ index 45a16d9de43703c2115dde85c9faae3a04be2a88..0cd76078218433b46c17f350e3ba6073 Maybe SerializerContext::WriteHostObject(Isolate* isolate, Local input) { MaybeLocal ret; -@@ -211,7 +240,12 @@ void SerializerContext::ReleaseBuffer(const FunctionCallbackInfo& args) { +@@ -209,9 +238,14 @@ void SerializerContext::ReleaseBuffer(const FunctionCallbackInfo& args) { + // Note: Both ValueSerializer and this Buffer::New() variant use malloc() + // as the underlying allocator. std::pair ret = ctx->serializer_.Release(); - auto buf = Buffer::New(ctx->env(), - reinterpret_cast(ret.first), +- auto buf = Buffer::New(ctx->env(), +- reinterpret_cast(ret.first), - ret.second); -+ ret.second, -+ [](char* data, void* hint){ -+ if (data) -+ GetAllocator()->Free(data, reinterpret_cast(hint)); -+ }, -+ reinterpret_cast(ctx->last_length_)); ++ std::unique_ptr bs = ++ v8::ArrayBuffer::NewBackingStore(reinterpret_cast(ret.first), ret.second, ++ [](void* data, size_t length, void* deleter_data) { ++ if (data) GetAllocator()->Free(reinterpret_cast(data), length); ++ }, nullptr); ++ Local 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}`); + } diff --git a/script/node-disabled-tests.json b/script/node-disabled-tests.json index d4335b6111d70..72e01357b087f 100644 --- a/script/node-disabled-tests.json +++ b/script/node-disabled-tests.json @@ -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",