Skip to content

Commit

Permalink
buffer: do not leak memory if buffer is too big
Browse files Browse the repository at this point in the history
A recent pull request changed this method to throw when the buffer was
too big, but this meant that the `free` finalizer would never get
called, leading to a memory leak.

Included is a test which provokes this behavior using `v8.serialize`.
Technically the behavior is allocator-dependent, but I suspect any
reasonable allocator will choose to free (or at the very least, reuse)
the 1 GiB memory.

Refs: nodejs#40243
  • Loading branch information
kvakil committed Jul 22, 2022
1 parent 0484022 commit d13bc57
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 0 deletions.
2 changes: 2 additions & 0 deletions src/node_buffer.cc
Expand Up @@ -497,6 +497,8 @@ MaybeLocal<Object> New(Environment* env,
if (length > kMaxLength) {
Isolate* isolate(env->isolate());
isolate->ThrowException(ERR_BUFFER_TOO_LARGE(isolate));
// Free the data now, so that it doesn't leak.
free(data);
return Local<Object>();
}
}
Expand Down
38 changes: 38 additions & 0 deletions test/pummel/test-v8-serialize-buffer-too-large.js
@@ -0,0 +1,38 @@
'use strict';
const common = require('../common');

// On IBMi, the rss memory always returns zero
if (common.isIBMi)
common.skip('On IBMi, the rss memory always returns zero');

const v8 = require('v8');
const {kMaxLength} = require('buffer');

const PADDING_STRING = `Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin non quam in diam laoreet rhoncus condimentum quis neque. Sed luctus arcu eget velit tincidunt rhoncus. Mauris eros libero, lobortis et dolor quis, interdum sagittis dolor. Maecenas sit amet nulla at risus ullamcorper gravida. Sed varius nulla vel faucibus accumsan. Sed luctus purus felis, sagittis vehicula justo sollicitudin sed. Duis laoreet lobortis condimentum. Nunc ac nisi quis dolor malesuada aliquet eget in quam.
Vivamus malesuada leo et nisi feugiat varius. Vivamus ut dapibus tellus. Nunc interdum metus eget odio accumsan efficitur. Donec ac nisl id justo ullamcorper porta. Aliquam molestie dictum purus, non tincidunt mi facilisis non. Praesent lorem felis, pretium at consectetur et, elementum nec nisi. Etiam placerat lorem at maximus vulputate. Donec sollicitudin pretium ligula. Curabitur eu porta leo, sit amet tempor leo. Vivamus venenatis massa metus, at tempor dolor pharetra vel. Cras eget turpis eu nisi elementum dapibus sit amet sit amet nibh. Aliquam rhoncus eros et mauris aliquam, rhoncus condimentum purus placerat. Nam mollis sollicitudin ante, non imperdiet nulla commodo vitae. Mauris sollicitudin quam ut ipsum dignissim, in mollis augue placerat. Morbi suscipit auctor hendrerit. Morbi dictum sagittis nulla nec posuere.
Suspendisse potenti. Proin vehicula est blandit, euismod velit sed, maximus augue. Nullam id rhoncus risus. Donec cursus lobortis porttitor. Donec fringilla, sem ac vehicula finibus, sem nisl ultricies leo, eu finibus sapien metus eu nibh. Fusce ut erat eu arcu aliquet tincidunt. Maecenas tristique enim non ante varius, quis semper justo efficitur. Integer maximus ultrices nisl at molestie.
Proin interdum, quam ut pellentesque congue, magna urna tristique felis, malesuada porta orci metus a purus. Morbi porttitor ex nec arcu mollis luctus. Ut quis tortor purus. Ut eu odio pharetra, fringilla ligula sit amet, sagittis lectus. Aenean ac quam vel ex mollis rutrum. Aliquam nulla leo, varius at mauris eu, porttitor egestas libero. Praesent sed feugiat augue, iaculis feugiat libero.
Suspendisse potenti. Suspendisse blandit ex quis nunc elementum pellentesque. Nam sagittis dui id faucibus faucibus. Pellentesque faucibus augue sit amet lorem cursus cursus. Pellentesque ut venenatis nisl, in placerat quam. Sed in enim at eros condimentum porttitor quis et lectus. Praesent sit amet pulvinar sapien. Quisque sodales mi ante, eu fermentum enim rhoncus sed. Praesent ac arcu eu erat cursus vulputate. Morbi cursus libero lectus, at tempus odio varius eget. Quisque finibus urna sed cursus rhoncus. Vivamus faucibus cursus imperdiet. Phasellus interdum sapien in odio rutrum, ut ornare turpis dictum. Sed mauris dui, molestie in odio nec, vulputate venenatis ex.`;

const i = 22;
const toSerialize = {};

for (let j = 0; j < 2 ** i; j++) {
toSerialize[j] = PADDING_STRING;
}

const assert = require('assert');

const rssBefore = process.memoryUsage.rss();
// Fail to serialize a few times.
assert.throws(() => v8.serialize(toSerialize));
assert.throws(() => v8.serialize(toSerialize));
assert.throws(() => v8.serialize(toSerialize));

// Check that (at least some of) the memory got freed.
const rssAfter = process.memoryUsage.rss();
assert(rssAfter - rssBefore <= 2 * kMaxLength);

0 comments on commit d13bc57

Please sign in to comment.