Bug 209929 - HeapSnapshotBuilder::analyzeNode() should filter out duplicate cells.
Summary: HeapSnapshotBuilder::analyzeNode() should filter out duplicate cells.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-04-02 14:18 PDT by Mark Lam
Modified: 2020-04-02 14:50 PDT (History)
6 users (show)

See Also:

Attachments
proposed patch. (3.59 KB, patch)
2020-04-02 14:27 PDT, Mark Lam 		keith_miller: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2020-04-02 14:18:18 PDT 
HeapSnapshot::finalize() assumes that its vector of cells are unique and have no duplicates.  HeapSnapshot::appendNode() expects to only be called once for a cell, and hence, will not be adding a duplicate cell.  It doesn't check for duplicates.

However, with the concurrent GC marker, there’s a racy chance that the same cell is visited more than once by SlotVisitor, and therefore calls HeapSnapshotBuilder::analyzeNode() (and HeapSnapshot::appendNode()) more than once for the same cell.

The easiest and cleanest fix for this is to simply keep a HashSet of appended cells in HeapSnapshotBuilder while it is building the snapshot.  We can then use the hash set to filter out already appended cells, and avoid adding duplicates to the HeapSnapshot.

<rdar://problem/60974478>
Comment 1 Mark Lam 2020-04-02 14:27:40 PDT 
Created attachment 395300 [details]
proposed patch.
Comment 2 Keith Miller 2020-04-02 14:45:19 PDT 
Comment on attachment 395300 [details]
proposed patch.

r=me.
Comment 3 Mark Lam 2020-04-02 14:50:06 PDT 
Thanks for the review.  Landed in r259418: <http://trac.webkit.org/r259418>.