Skip to content

Commit

Permalink
Fix issue 41311 - Assert failure ephemeral_heap_segment->saved_commit…
Browse files Browse the repository at this point in the history
…ted == heap_segment_committed (dotnet#41441)

This is an interaction problem between gradual decommit and entering a no GC region.

Details:
 - When we enter a no GC region, we are manipulating our commit goals outside of GC, we usually are planning to commit more, so it makes sense to stop any gradual decommit in progress.
- Even if we need to do a GC in preparation for a no GC region, any decommit target established by decommit_ephemeral_segment_pages is likely to be invalid, so we do an early out.
- add an assert to grow_heap_segment so we can catch future issues in this area better.

We may want to trigger the gradual decommit logic in set_allocations_for_no_gc if we discover that after accounting for the planned allocation, we still have substantial decommits. I'm leaving this for a later PR though.
  • Loading branch information
PeterSolMS authored Aug 31, 2020
1 parent 007a0e9 commit a33ad57
Showing 1 changed file with 12 additions and 2 deletions.
14 changes: 12 additions & 2 deletions src/coreclr/src/gc/gc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5705,6 +5705,7 @@ void gc_heap::gc_thread_function ()
END_TIMING(suspend_ee_during_log);

proceed_with_gc_p = TRUE;
gradual_decommit_in_progress_p = FALSE;

if (!should_proceed_with_gc())
{
Expand Down Expand Up @@ -11506,6 +11507,15 @@ BOOL gc_heap::grow_heap_segment (heap_segment* seg, uint8_t* high_address, bool*

assert (heap_segment_committed (seg) <= heap_segment_reserved (seg));
assert (high_address <= heap_segment_committed (seg));

#ifdef MULTIPLE_HEAPS
// we should never increase committed beyond decommit target when gradual
// decommit is in progress - if we do, this means commit and decommit are
// going on at the same time.
assert (!gradual_decommit_in_progress_p ||
(seg != ephemeral_heap_segment) ||
(heap_segment_committed (seg) <= heap_segment_decommit_target (seg)));
#endif // MULTIPLE_HEAPS
}

return !!ret;
Expand Down Expand Up @@ -32458,7 +32468,7 @@ void gc_heap::trim_youngest_desired_low_memory()

void gc_heap::decommit_ephemeral_segment_pages()
{
if (settings.concurrent || use_large_pages_p)
if (settings.concurrent || use_large_pages_p || (settings.pause_mode == pause_no_gc))
{
return;
}
Expand Down Expand Up @@ -32497,7 +32507,7 @@ void gc_heap::decommit_ephemeral_segment_pages()
decommit_target += target_decrease * 2 / 3;
}

heap_segment_decommit_target(ephemeral_heap_segment) = decommit_target;
heap_segment_decommit_target (ephemeral_heap_segment) = decommit_target;

#ifdef MULTIPLE_HEAPS
if (decommit_target < heap_segment_committed (ephemeral_heap_segment))
Expand Down

0 comments on commit a33ad57

Please sign in to comment.