Skip to content
This repository has been archived by the owner on Mar 25, 2018. It is now read-only.

Commit

Permalink
[heap] Ensure that the sweeper does not lose unswept pages.
Browse files Browse the repository at this point in the history
This fixes a race between the sweeper and the array buffer tracker
that causes the sweeper to skip unswept pages.

The scenario:
1. Mark-compact GC adds page p to the sweeping_list_ of the sweeper.
2. GC finishes, the main thread starts executinng JS.
3. The main thread takes p->mutex to unregister an array buffer.
4. A sweeper thread removes p from the sweeping_list_ and tries to
   take p->mutex. The try fails. The sweeper drops p and continues
   to the next page.
5. During selection of evacuation candidate in the next GC we hit
   page->SweepingDone() assert.

BUG=chromium:650314

Review-Url: https://codereview.chromium.org/2484153004
Cr-Commit-Position: refs/heads/master@{#40857}
  • Loading branch information
ulan authored and Commit bot committed Nov 9, 2016
1 parent ac183d4 commit b621987
Showing 1 changed file with 10 additions and 11 deletions.
21 changes: 10 additions & 11 deletions src/heap/mark-compact.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3741,12 +3741,12 @@ int MarkCompactCollector::Sweeper::ParallelSweepSpace(AllocationSpace identity,
int MarkCompactCollector::Sweeper::ParallelSweepPage(Page* page,
AllocationSpace identity) {
int max_freed = 0;
if (page->mutex()->TryLock()) {
{
base::LockGuard<base::Mutex> guard(page->mutex());
// If this page was already swept in the meantime, we can return here.
if (page->concurrent_sweeping_state().Value() != Page::kSweepingPending) {
page->mutex()->Unlock();
return 0;
}
if (page->SweepingDone()) return 0;
DCHECK_EQ(Page::kSweepingPending,
page->concurrent_sweeping_state().Value());
page->concurrent_sweeping_state().SetValue(Page::kSweepingInProgress);
const Sweeper::FreeSpaceTreatmentMode free_space_mode =
Heap::ShouldZapGarbage() ? ZAP_FREE_SPACE : IGNORE_FREE_SPACE;
Expand All @@ -3755,6 +3755,7 @@ int MarkCompactCollector::Sweeper::ParallelSweepPage(Page* page,
} else {
max_freed = RawSweep(page, REBUILD_FREE_LIST, free_space_mode);
}
DCHECK(page->SweepingDone());

// After finishing sweeping of a page we clean up its remembered set.
if (page->typed_old_to_new_slots()) {
Expand All @@ -3763,13 +3764,11 @@ int MarkCompactCollector::Sweeper::ParallelSweepPage(Page* page,
if (page->old_to_new_slots()) {
page->old_to_new_slots()->FreeToBeFreedBuckets();
}
}

{
base::LockGuard<base::Mutex> guard(&mutex_);
swept_list_[identity].Add(page);
}
page->concurrent_sweeping_state().SetValue(Page::kSweepingDone);
page->mutex()->Unlock();
{
base::LockGuard<base::Mutex> guard(&mutex_);
swept_list_[identity].Add(page);
}
return max_freed;
}
Expand Down

0 comments on commit b621987

Please sign in to comment.