Skip to content

Commit

Permalink
don't call GetHeap to get the heap in wait_for_gc_done (dotnet#107073)
Browse files Browse the repository at this point in the history
we see the assert in GCHeap::GetHeap when called by wait_for_gc_done -

GCHeap* GCHeap::GetHeap (int n)
{
    assert (n < gc_heap::n_heaps);
    return gc_heap::g_heaps[n]->vm_heap;
}

when HC change is in progress and we are reducing the HC. this assert is actually unnecessary when called by wait_for_gc_done but is necessary when called by other methods like balance_heaps (those are in coop mode so they cannot observe the HC transition). so I just changed to getting the heap from g_heaps instead of calling GetHeap.

previously I thought this could actually be a functional problem but after doing more thinking I think this is just a benign assert. if we are observing the transition from the old HC to the new HC in wait_for_gc_done and we happen to be waiting on a heap that's decommissioned, decommission_heap sets its heap's gc_done_event so what will happen is wait_for_gc_done will wake up from waiting as soon decommission_heap is executed. at this time n_heaps is also updated. so when it gets to the next iteration in the while loop it will get a valid heap's gc_done_event to wait on.
  • Loading branch information
Maoni0 authored Sep 6, 2024
1 parent 66b9b68 commit ce8f49e
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/coreclr/gc/gc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14787,7 +14787,7 @@ gc_heap::wait_for_gc_done(int32_t timeOut)
while (gc_heap::gc_started)
{
#ifdef MULTIPLE_HEAPS
wait_heap = GCHeap::GetHeap(heap_select::select_heap(NULL))->pGenGCHeap;
wait_heap = g_heaps[heap_select::select_heap(NULL)];
dprintf(2, ("waiting for the gc_done_event on heap %d", wait_heap->heap_number));
#endif // MULTIPLE_HEAPS

Expand Down

0 comments on commit ce8f49e

Please sign in to comment.