Commit b6219871 authored by ulan's avatar ulan Committed by Commit bot

[heap] Ensure that the sweeper does not lose unswept pages.

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}
parent ac183d49
......@@ -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;
......@@ -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()) {
......@@ -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;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment