[cDAC] Use specific exception for virtual read failures #118895
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new specific exception type VirtualReadException to replace the generic InvalidOperationException for virtual memory read failures in the cDAC diagnostic component. This provides better error handling specificity and clearer debugging information.
Key changes:
- Introduces
VirtualReadExceptionclass with appropriate HResult for virtual read failures - Replaces
InvalidOperationExceptioncatches/throws withVirtualReadExceptionthroughout the codebase - Updates method signatures and documentation to reflect the new exception type
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| VirtualReadException.cs | New exception class specifically for virtual read failures with proper HResult |
| CorDbHResults.cs | Made class public and moved to correct namespace for wider accessibility |
| ContractDescriptorTarget.cs | Updated read methods to throw VirtualReadException and updated method signatures |
| Target.cs | Added exception documentation to abstract methods and updated Write method signature |
| ClrDataModule.cs | Updated catch block to handle VirtualReadException instead of InvalidOperationException |
| SOSDacImpl.cs | Removed redundant catch block for InvalidOperationException |
| PrintfStressMessageFormatter.cs | Updated exception handling for string read operations |
| AMD64Unwinder.cs | Updated exception handling for memory read failures |
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Target.cs
Outdated
Show resolved
Hide resolved
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Target.cs
Outdated
Show resolved
Hide resolved
|
Tagging subscribers to this area: @steveisok, @dotnet/dotnet-diag |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#118702 (review)
Modifies the
Targetto throwVirtualReadExceptionwhen there is a failure when doing a virtual read. This has an HResult ofCORDBG_E_READVIRTUAL_FAILUREmatching the DACs return in the same scenario.Note, global reads continue to throw
InvalidOperationExceptionas these don't trigger a virtual read, but return a value from a dictionary.