From c6b9340b8952a041373f694c6ee2f22f447464de Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Thu, 26 Jan 2023 07:43:57 +0100 Subject: [PATCH] chore: fix memory leak in `v8.serialize()` (#37021) chore: fix memory leak in v8.serialize() --- .../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 59286b5160b9..f3e585f58e1d 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 9855b3539c62..d3da23d7a491 100644 --- a/patches/node/support_v8_sandboxed_pointers.patch +++ b/patches/node/support_v8_sandboxed_pointers.patch @@ -155,7 +155,7 @@ index efbdbeabf5a6afb658cbdc7888f94952e55f4f71..8b37639361e8902d7e1481071d3ec24b // 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 bcfc42d59acd..d3bba00b0cc1 100644 --- a/script/node-disabled-tests.json +++ b/script/node-disabled-tests.json @@ -137,7 +137,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",