Skip to content

Commit cacff07

Browse files
liuxingbaoyuRafaelGSS
authored andcommitted
src: fix memory leak for v8.serialize
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>
1 parent 1237c74 commit cacff07

File tree

2 files changed

+36
-2
lines changed

2 files changed

+36
-2
lines changed

src/node_buffer.cc

+14-2
Original file line numberDiff line numberDiff line change
@@ -502,8 +502,20 @@ MaybeLocal<Object> New(Environment* env,
502502
}
503503
}
504504

505-
auto free_callback = [](char* data, void* hint) { free(data); };
506-
return New(env, data, length, free_callback, nullptr);
505+
EscapableHandleScope handle_scope(env->isolate());
506+
507+
auto free_callback = [](void* data, size_t length, void* deleter_data) {
508+
free(data);
509+
};
510+
std::unique_ptr<BackingStore> bs =
511+
v8::ArrayBuffer::NewBackingStore(data, length, free_callback, nullptr);
512+
513+
Local<ArrayBuffer> ab = v8::ArrayBuffer::New(env->isolate(), std::move(bs));
514+
515+
Local<Object> obj;
516+
if (Buffer::New(env, ab, 0, length).ToLocal(&obj))
517+
return handle_scope.Escape(obj);
518+
return Local<Object>();
507519
}
508520

509521
namespace {
+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
'use strict';
2+
// Flags: --expose-gc
3+
4+
require('../common');
5+
const v8 = require('v8');
6+
const assert = require('assert');
7+
8+
const before = process.memoryUsage.rss();
9+
10+
for (let i = 0; i < 1000000; i++) {
11+
v8.serialize('');
12+
}
13+
14+
global.gc();
15+
16+
const after = process.memoryUsage.rss();
17+
18+
if (process.config.variables.asan) {
19+
assert(after < before * 10, `asan: before=${before} after=${after}`);
20+
} else {
21+
assert(after < before * 2, `before=${before} after=${after}`);
22+
}

0 commit comments

Comments
 (0)