[NativeAOT] Print OOM message before Abort() on Linux#125311
Conversation
Co-authored-by: agocke <515774+agocke@users.noreply.github.com>
|
Please add a unit test to src/tests |
Co-authored-by: agocke <515774+agocke@users.noreply.github.com>
Added a new |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
🤖 Copilot Code Review — PR #125311Note This review was generated by GitHub Copilot using claude-opus-4.6, with additional analysis from claude-haiku-4.5 and gpt-5.3-codex. Holistic AssessmentMotivation: The fix is clearly justified. Before this PR, NativeAOT processes that ran out of memory and hit the Approach: The approach is sound—adding a best-effort stderr write in the minimal fail-fast path, wrapped in try/catch to handle the case where even that small allocation fails. The additional hardening of Summary: ✅ LGTM. The code changes are correct, well-scoped, and follow the defensive patterns appropriate for a fatal error path. The test is well-designed with proper subprocess isolation and covers both allocation strategies. Two minor observations are noted below but neither is merge-blocking. Detailed Findings✅ Correctness — minimalFailFast path restructuringThe control flow change from The message ✅ Thread Safety —
|
…y for CoreCLR/NativeAOT
MichalStrehovsky
left a comment
There was a problem hiding this comment.
LGTM otherwise. We should run native AOT outerloop on this, Pri0 don't run by default.
| // report that OOM is the reason for the crash. | ||
| try | ||
| { | ||
| Internal.Console.Error.Write("Process terminated. System.OutOfMemoryException"); |
There was a problem hiding this comment.
Any reason not to take the suggestion? This would match the existing Process is terminating due to StackOverflowException.. If we have full control over the string, something readable is nicer.
| Internal.Console.Error.Write("Process terminated. System.OutOfMemoryException"); | |
| Internal.Console.Error.Write("Process is terminating due to OutOfMemoryException."); |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
On Linux, NativeAOT processes terminating due to
OutOfMemoryException(e.g. withDOTNET_GCHeapHardLimitset) printed onlyAbortedwith no diagnostic context.Root cause
RuntimeExceptionHelpers.FailFastdetects the preallocated OOM exception viaminimalFailFast = (exception == PreallocatedOutOfMemoryException.Instance)and skips all stderr output to avoid heap allocations — leaving the user with no indication of why the process died.Fix
In the
minimalFailFastpath, print a hardcoded OOM message to stderr before callingAbort(). The write is wrapped intry/catch {}so a secondary allocation failure silently falls through to the existing abort path.Before:
AbortedAfter:
Process terminated. System.OutOfMemoryExceptionTest
Added a new
OomHandlingsmoke test insrc/tests/baseservices/exceptions/OutOfMemoryException/. The test spawns itself as a subprocess withDOTNET_GCHeapHardLimit=0x20000000(32 MB) set, waits for the subprocess to run out of memory, and verifies thatOutOfMemoryExceptionappears in stderr. This covers both the preallocated OOM path (the fix) and the existing unhandled-exception path. The test is skipped on mobile and browser platforms that do not support process spawning.Original prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.