Changeset 207230 in webkit


Ignore:
Timestamp:
Oct 12, 2016 12:01:21 PM (7 years ago)
Author:
fpizlo@apple.com
Message:

REGRESSION (r207179): ASSERTION FAILED: node.cell != previousCell
https://bugs.webkit.org/show_bug.cgi?id=163337

Reviewed by Mark Lam.

It turns out that HeapSnapshot was not down with revisiting. The concurrent GC is going to be
built around the idea that we can revisit objects many times. This means that any action that
should only take place once per object must check the object's state. This fixes the snapshot
code to do this.

While writing this code, I realized that we're actually doing this check incorrectly, so I
filed bug 163343. That bug requires a race, so we aren't going to see it yet.

  • heap/HeapSnapshot.cpp:

(JSC::HeapSnapshot::finalize):

  • heap/SlotVisitor.cpp:

(JSC::SlotVisitor::appendToMarkStack):
(JSC::SlotVisitor::visitChildren):

Location:
trunk/Source/JavaScriptCore
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r207229 r207230  
     12016-10-12  Filip Pizlo  <fpizlo@apple.com>
     2
     3        REGRESSION (r207179): ASSERTION FAILED: node.cell != previousCell
     4        https://bugs.webkit.org/show_bug.cgi?id=163337
     5
     6        Reviewed by Mark Lam.
     7       
     8        It turns out that HeapSnapshot was not down with revisiting. The concurrent GC is going to be
     9        built around the idea that we can revisit objects many times. This means that any action that
     10        should only take place once per object must check the object's state. This fixes the snapshot
     11        code to do this.
     12       
     13        While writing this code, I realized that we're actually doing this check incorrectly, so I
     14        filed bug 163343. That bug requires a race, so we aren't going to see it yet.
     15
     16        * heap/HeapSnapshot.cpp:
     17        (JSC::HeapSnapshot::finalize):
     18        * heap/SlotVisitor.cpp:
     19        (JSC::SlotVisitor::appendToMarkStack):
     20        (JSC::SlotVisitor::visitChildren):
     21
    1222016-10-12  Joseph Pecoraro  <pecoraro@apple.com>
    223
  • trunk/Source/JavaScriptCore/heap/HeapSnapshot.cpp

    r201520 r207230  
    122122        ASSERT(node.cell);
    123123        ASSERT(!(reinterpret_cast<intptr_t>(node.cell) & CellToSweepTag));
    124         if (previousCell)
     124        if (node.cell == previousCell) {
     125            dataLog("Seeing same cell twice: ", RawPointer(previousCell), "\n");
    125126            ASSERT(node.cell != previousCell);
     127        }
    126128        previousCell = node.cell;
    127129    }
  • trunk/Source/JavaScriptCore/heap/SlotVisitor.cpp

    r207179 r207230  
    238238   
    239239    m_stack.append(cell);
    240 
    241     if (UNLIKELY(m_heapSnapshotBuilder))
    242         m_heapSnapshotBuilder->appendNode(cell);
    243240}
    244241
     
    319316        cell->methodTable()->visitChildren(const_cast<JSCell*>(cell), *this);
    320317        break;
     318    }
     319   
     320    if (UNLIKELY(m_heapSnapshotBuilder)) {
     321        if (cell->cellState() != CellState::OldBlack)
     322            m_heapSnapshotBuilder->appendNode(const_cast<JSCell*>(cell));
    321323    }
    322324}
Note: See TracChangeset for help on using the changeset viewer.