From ce8f49e658d4a31e5708f0c06efa39784ee7c1ad Mon Sep 17 00:00:00 2001 From: Maoni Stephens Date: Thu, 5 Sep 2024 23:13:57 -0700 Subject: [PATCH] don't call GetHeap to get the heap in wait_for_gc_done (#107073) 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. --- src/coreclr/gc/gc.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 20cfda9282651..58eb8614b2f59 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -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