Skip to content

Conversation

@alexey-zakharov
Copy link

@alexey-zakharov alexey-zakharov commented Mar 15, 2024

Fixed crash in gc_heap::walk_heap_per_heap when walking frozen segments

Motivation:

GC Heap diagnostics crashes after AssemblyLoadContext unloading with the following callstack

[Inline Frame] coreclr.dll!WKS::my_get_size(Object *) Line 11572	C++	Symbols loaded.
>	coreclr.dll!WKS::gc_heap::walk_heap_per_heap(bool(*)(Object *, void *) fn, void * context, int gen_number, int walk_large_object_heap_p) Line 51974	C++	Symbols loaded.
 	coreclr.dll!GCProfileWalkHeapWorker(int fProfilerPinned, int fShouldWalkHeapRootsForEtw, int fShouldWalkHeapObjectsForEtw) Line 727	C++	Symbols loaded.
 	coreclr.dll!GCToEEInterface::DiagGCEnd(unsigned __int64 fConcurrent, int index, int gen, bool reason) Line 818	C++	Symbols loaded.
 	coreclr.dll!WKS::gc_heap::do_post_gc() Line 50183	C++	Symbols loaded.
 	coreclr.dll!WKS::gc_heap::gc1() Line 22865	C++	Symbols loaded.
 	[Inline Frame] coreclr.dll!GCToOSInterface::GetLowPrecisionTimeStamp() Line 1091	C++	Symbols loaded.
 	coreclr.dll!WKS::gc_heap::garbage_collect(int n) Line 24329	C++	Symbols loaded.
 	coreclr.dll!WKS::GCHeap::GarbageCollectGeneration(unsigned int gen, gc_reason reason) Line 50526	C++	Symbols loaded.
 	coreclr.dll!WKS::GCHeap::GarbageCollect(int generation, bool low_memory_p, int mode) Line 49681	C++	Symbols loaded.
 	coreclr.dll!ETW::GCLog::ForceGCForDiagnostics() Line 492	C++	Symbols loaded.
 	[Inline Frame] coreclr.dll!ETW::GCLog::ForceGC(__int64) Line 431	C++	

The issue can be reproduced with PerfView tool without Unity Editor enabling COR_PRF_MONITOR_GC flag at startup - tool crashes on GC Heap dump of Unity Editor after we successfully unloaded an AssemblyLoadContext.

I was suspecting heap corruption, but was not able to gather any proof. Comparison with gc::verify_heap yielded that heap verification uses heap_segment_in_range_p method to get the heap segment to iterate over. heap_segment_in_range_p checks that segment is not readonly/frozen (heap_segment_flags_readonly flag) or if it is “in range” (heap_segment_flags_inrange flag).

Changes:

It is a bit unclear to full extent what USE_RANGES feature do, however walking heap for diagnostics is different than Heap Verify functionality, so I suggest align segments walking strategy and ask upstream whether or not this is expected to be same or not.

@alexey-zakharov alexey-zakharov marked this pull request as ready for review March 15, 2024 16:30
@alexey-zakharov alexey-zakharov requested a review from joncham March 15, 2024 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants