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

fix: Fix takeHeapSnapshot() truncation bug #58

Merged
merged 1 commit into from
Jan 23, 2018
Merged

fix: Fix takeHeapSnapshot() truncation bug #58

merged 1 commit into from
Jan 23, 2018

Conversation

bnoordhuis
Copy link
Member

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

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
Copy link
Collaborator

@jkrems jkrems left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this! Sorry, was out sick and then tied down with work for the past week...

@jkrems jkrems merged commit 41b39ee into nodejs:master Jan 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants