-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Defer DOTNET_DbgEnableMiniDump error message until dump creation #122986
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…Line Move the error message from PalCreateDumpInitialize to BuildCreateDumpCommandLine. This prevents the error from appearing during initialization when createdump binary doesn't exist, and instead only shows it when a dump is actually being created. This allows DOTNET_DbgEnableMiniDump to be set even when crossgen is involved. Co-authored-by: steveisok <471438+steveisok@users.noreply.github.com>
Co-authored-by: steveisok <471438+steveisok@users.noreply.github.com>
|
@copilot I'd like to keep more in the spirit of the original logic. Rather than store the path regardless if the file exists, bring back the old logic and just move where the error message gets printed. |
…mandLine Keep the original logic where the path is not stored if the file doesn't exist. Only move the fprintf from PalCreateDumpInitialize to BuildCreateDumpCommandLine. Co-authored-by: steveisok <471438+steveisok@users.noreply.github.com>
Updated in 6d701ff. The original logic is now preserved - the path is not stored if the file doesn't exist. The fprintf was simply moved from PalCreateDumpInitialize to BuildCreateDumpCommandLine. |
Co-authored-by: steveisok <471438+steveisok@users.noreply.github.com>
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.
Pull request overview
This PR defers an error message about a missing createdump binary from runtime initialization to actual dump creation time. This fixes an issue where setting DOTNET_DbgEnableMiniDump in crossgen scenarios would produce spurious error messages even when no dump would be created.
Key Changes:
- Error message moved from
PalCreateDumpInitialize()toBuildCreateDumpCommandLine()to only appear when explicitly generating a dump - Added
free(program)to prevent memory leak when createdump binary doesn't exist
|
An alternative fix could be to update the crossgen2 task (the one that invokes crossgen2) to ignore stderr. The problem the message is causing is that if MSBuild sees writes to stderr, it will fail the tool invocation task. Otherwise the message is not harmful, it might even be helpful. |
Not a bad suggestion. The value of having a message at startup is that you know you aren't in a good state ahead of time as opposed to realizing it only when the crash occurs. @hoyosjs - what do you think? |
Add a new global flag g_warnCreateDumpMissing to distinguish between: - DOTNET_DbgEnableMiniDump not set (no warning needed) - DOTNET_DbgEnableMiniDump set but createdump missing (warning on dump creation) Set the flag in PalCreateDumpInitialize when createdump is not found. Check the flag in both dump creation code paths: - BuildCreateDumpCommandLine (called by PalGenerateCoreDump) - PalCreateCrashDumpIfEnabled (called on crash) This ensures the warning only appears when actually trying to create a dump, not during initialization. Co-authored-by: noahfalk <6243776+noahfalk@users.noreply.github.com>
Extract the error message into g_createDumpMissingMessage constant to avoid duplication and make future updates easier. Co-authored-by: noahfalk <6243776+noahfalk@users.noreply.github.com>
|
I think such a task invocation can set to ignore stderr - if crossgen never tries to write to err without also returning error that is a good approach for the immediate issue. As for the createdump issue - I still think the error should only be displayed at the crash point. Startup could've happened long ago and the logs should show this near the crash point. Also, I wonder if it's better to always perform the check at crash time to allow users to drop createdump after deployment as needed. |
|
Tagging subscribers to this area: @steveisok, @dotnet/dotnet-diag |
Remove g_warnCreateDumpMissing flag and stat() check during initialization. Instead, detect the missing createdump binary when execv() returns ENOENT error and print the error message at that point. This defers the check until dump creation time, avoiding unnecessary file system checks during initialization. Co-authored-by: noahfalk <6243776+noahfalk@users.noreply.github.com>
jkotas
left a comment
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!
lol thanks. I need to look harder. |

Description
The error message "DOTNET_DbgEnableMiniDump is set and the createdump binary does not exist" was being printed during runtime initialization, even when no dump would be created. This prevented setting the environment variable in crossgen scenarios where createdump isn't shipped.
Changes:
stat()) during initialization inPalCreateDumpInitialize()to avoid unnecessary file system operationsexecv()returnsENOENTerror inCreateCrashDump()Customer Impact
Customers using crossgen tooling cannot set
DOTNET_DbgEnableMiniDumpwithout spurious error messages during builds. This affects debugging workflows in environments where dump generation is conditionally needed.Regression
No. This is an existing issue with the environment variable handling.
Testing
Built NativeAOT runtime successfully with changes. Verified:
Risk
Low. Changes eliminate unnecessary file system checks during initialization and defer error detection to the point where the binary is actually needed (execv call). The actual dump creation logic and all success paths remain unchanged. Error messages now appear only when dumps are actually attempted, preventing spurious warnings during initialization.
Original prompt
DOTNET_DbgEnableMiniDumpwhen crossgen is involved somewhere in the build #122982💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.