Skip to content
This repository was archived by the owner on Feb 1, 2022. It is now read-only.

Commit 94f0bf9

Browse files
bnoordhuisJan Krems
authored and
Jan Krems
committed
fix: Fix takeHeapSnapshot() truncation bug
Fix a logic bug in `takeHeapSnapshot()` where it interpreted the item count as the byte count, resulting in truncated snapshots. This change hinges on the observation that the completion callback always comes after any and all `'addHeapSnapshotChunk'` events. I'm 95% sure after digging into the V8 inspector and its integration with Node.js that the assumption above is correct. If it turns out I'm mistaken, then we will most likely treat that as a bug in Node.js, not node-inspect. Fixes: #56
1 parent c07adb1 commit 94f0bf9

File tree

2 files changed

+44
-10
lines changed

2 files changed

+44
-10
lines changed

lib/internal/inspect_repl.js

+10-10
Original file line numberDiff line numberDiff line change
@@ -900,10 +900,8 @@ function createRepl(inspector) {
900900
return new Promise((resolve, reject) => {
901901
const absoluteFile = Path.resolve(filename);
902902
const writer = FS.createWriteStream(absoluteFile);
903-
let totalSize;
904903
let sizeWritten = 0;
905904
function onProgress({ done, total, finished }) {
906-
totalSize = total;
907905
if (finished) {
908906
print('Heap snaphost prepared.');
909907
} else {
@@ -913,13 +911,18 @@ function createRepl(inspector) {
913911
function onChunk({ chunk }) {
914912
sizeWritten += chunk.length;
915913
writer.write(chunk);
916-
print(`Writing snapshot: ${sizeWritten}/${totalSize}`, true);
917-
if (sizeWritten >= totalSize) {
918-
writer.end();
914+
print(`Writing snapshot: ${sizeWritten}`, true);
915+
}
916+
function onResolve() {
917+
writer.end(() => {
919918
teardown();
920919
print(`Wrote snapshot: ${absoluteFile}`);
921920
resolve();
922-
}
921+
});
922+
}
923+
function onReject(error) {
924+
teardown();
925+
reject(error);
923926
}
924927
function teardown() {
925928
HeapProfiler.removeListener(
@@ -932,10 +935,7 @@ function createRepl(inspector) {
932935

933936
print('Heap snapshot: 0/0', true);
934937
HeapProfiler.takeHeapSnapshot({ reportProgress: true })
935-
.then(null, (error) => {
936-
teardown();
937-
reject(error);
938-
});
938+
.then(onResolve, onReject);
939939
});
940940
},
941941

test/cli/heap-profiler.test.js

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
'use strict';
2+
const { test } = require('tap');
3+
const { readFileSync, unlinkSync } = require('fs');
4+
5+
const startCLI = require('./start-cli');
6+
const filename = 'node.heapsnapshot';
7+
8+
function cleanup() {
9+
try {
10+
unlinkSync(filename);
11+
} catch (_) {
12+
// Ignore.
13+
}
14+
}
15+
16+
cleanup();
17+
18+
test('Heap profiler take snapshot', (t) => {
19+
const cli = startCLI(['examples/empty.js']);
20+
21+
function onFatal(error) {
22+
cli.quit();
23+
throw error;
24+
}
25+
26+
// Check that the snapshot is valid JSON.
27+
return cli.waitForInitialBreak()
28+
.then(() => cli.waitForPrompt())
29+
.then(() => cli.command('takeHeapSnapshot()'))
30+
.then(() => JSON.parse(readFileSync(filename, 'utf8')))
31+
.then(() => cleanup())
32+
.then(() => cli.quit())
33+
.then(null, onFatal);
34+
});

0 commit comments

Comments
 (0)