From 36163e7af7dd7f3c15097818cd337763ec26d0e4 Mon Sep 17 00:00:00 2001 From: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com> Date: Tue, 3 May 2022 03:45:47 +0800 Subject: [PATCH] src: fix memory leak for v8.serialize When Buffer::New passes in existing data, it cannot be garbage collected in js synchronous execution. Fixes: https://github.com/nodejs/node/issues/40828 Refs: https://github.com/nodejs/node/issues/38300 PR-URL: https://github.com/nodejs/node/pull/42695 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Minwoo Jung --- src/node_buffer.cc | 16 ++++++++++++++-- test/parallel/test-v8-serialize-leak.js | 22 ++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-v8-serialize-leak.js diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 215bd8003aabe1..dcf5d84ca34a2e 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -502,8 +502,20 @@ MaybeLocal New(Environment* env, } } - auto free_callback = [](char* data, void* hint) { free(data); }; - return New(env, data, length, free_callback, nullptr); + EscapableHandleScope handle_scope(env->isolate()); + + auto free_callback = [](void* data, size_t length, void* deleter_data) { + free(data); + }; + std::unique_ptr bs = + v8::ArrayBuffer::NewBackingStore(data, length, free_callback, nullptr); + + Local ab = v8::ArrayBuffer::New(env->isolate(), std::move(bs)); + + Local obj; + if (Buffer::New(env, ab, 0, length).ToLocal(&obj)) + return handle_scope.Escape(obj); + return Local(); } namespace { diff --git a/test/parallel/test-v8-serialize-leak.js b/test/parallel/test-v8-serialize-leak.js new file mode 100644 index 00000000000000..4e3a1d96f8ad6e --- /dev/null +++ b/test/parallel/test-v8-serialize-leak.js @@ -0,0 +1,22 @@ +'use strict'; +// Flags: --expose-gc + +require('../common'); +const v8 = require('v8'); +const assert = require('assert'); + +const before = process.memoryUsage.rss(); + +for (let i = 0; i < 1000000; i++) { + v8.serialize(''); +} + +global.gc(); + +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}`); +}