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: nodejs#40828
Refs: nodejs#38300
  • Loading branch information
liuxingbaoyu committed Apr 11, 2022
1 parent 4508c8c commit 809386a
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/node_serdes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,10 @@ 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(),
auto buf = Buffer::Copy(ctx->env(),
reinterpret_cast<char*>(ret.first),
ret.second);
free(ret.first);

if (!buf.IsEmpty()) {
args.GetReturnValue().Set(buf.ToLocalChecked());
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(before * 10 > after, `asan: before=${before} after=${after}`);
} else {
assert(after - before < 1024 * 1024 * 10, `before=${before} after=${after}`);
}

0 comments on commit 809386a

Please sign in to comment.