Skip to content

Commit

Permalink
src: fix memory leak for v8.serialize
Browse files Browse the repository at this point in the history
When Buffer::New passes in existing data,
it cannot be garbage collected in js synchronous execution.

Fixes: #40828
Refs: #38300

PR-URL: #42695
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
  • Loading branch information
liuxingbaoyu authored and targos committed Jul 12, 2022
1 parent d9846b4 commit 774b112
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 2 deletions.
16 changes: 14 additions & 2 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -502,8 +502,20 @@ MaybeLocal<Object> 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<BackingStore> bs =
v8::ArrayBuffer::NewBackingStore(data, length, free_callback, nullptr);

Local<ArrayBuffer> ab = v8::ArrayBuffer::New(env->isolate(), std::move(bs));

Local<Object> obj;
if (Buffer::New(env, ab, 0, length).ToLocal(&obj))
return handle_scope.Escape(obj);
return Local<Object>();
}

namespace {
Expand Down
22 changes: 22 additions & 0 deletions test/parallel/test-v8-serialize-leak.js
Original file line number Diff line number Diff line change
@@ -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}`);
}

0 comments on commit 774b112

Please sign in to comment.