-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Fixes issue #21484 createdump segfaults #21534
Conversation
Issue #21484: createdump segfaults with ASP.NET app The problem is the ClrDataModule Request faulted on a dynamic module getting the file layout flag. Fixed the Request code not get the file layout and in the crash dump code skip any dynamic modules. Issue #21485: fix EnumProcessModules hPseudoCurrentProcess bug.
Description The createdump utility segfaults with ASP.NET Core 2.2 applications. Customer Impact Customers can't create a core dump and diagnostic there ASP.NET app or any .NET Core that has dynamic modules. Regression? Not a regression. Risk Low. Affects Linux only. CR Jan Kotas has review this. |
@Maoni0, does it make sense that GC heap globals like ephemeral_heap_segment and finalize_queue were null for some apps? |
ephemeral_heap_segment and finalize_queue are initialized during CLR init (in InitializeGarbageCollector) so if the runtime is loaded they should not be null. note that in SVR GC these are per heap members whereas in WKS these are statics (ie, dac global vars) |
3fcd10c
to
dd1cac2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
GC heap globals like ephemeral_heap_segment and finalize_queue are null/invalid for a server GC. Add a check to skip the workstation GC memory enumeration if server. The server memory enumeration already skips if workstation GC. The dynamic module fix is still needed, createdump would have still hit this problem after the GC heap fix. Report the entire generation table in EnumWksGlobalMemoryRegions and EnumSvrGlobalMemoryRegions (dotnet#20233).
dd1cac2
to
f25aa3c
Compare
@dotnet-bot test Windows_NT x64 Formatting |
@dotnet-bot test Ubuntu arm Cross Checked Innerloop Build and Test |
@jamshedd what about ship room approval for this 2.2 PR? |
@russk to follow up on test coverage with @mikem8361 . |
I tested createdump manually on ubuntu and fedora against a webapp. I also ran our diagnostictests against the changes and everything succeeds. The diagnostic tests contain createdump tests. |
@mikem8361 Are you adding/have you added test coverage that would have caught this? I assume there was a hole before; shiproom would like to know if it is plugged. |
I haven't added any additional test coverage yet. I created issue #21892 to track this. |
@mikem8361 ok, so it sounds like in any case any additional test coverage wouldn't be included in this change since it wouldn't be in the coreclr repo. Thanks for the additional issue to track that. |
Approved for 2.2.2. |
Issue #21484: createdump segfaults with ASP.NET app
The problem is the ClrDataModule Request faulted on a dynamic module
getting the file layout flag.
Fixed the Request code not get the file layout and in the crash dump
code skip any dynamic modules.
Issue #21485: fix EnumProcessModules hPseudoCurrentProcess bug.