[WIP] Test fix for diagnostics failure - #131777
Draft
rcj1 wants to merge 1 commit into
Draft
Conversation
rcj1
added
the
NO-MERGE
|
Azure Pipelines: Successfully started running 4 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts legacy cDAC vs native DAC behavior validation and exception handling in method-instance enumeration paths used by SOS-style COM interfaces, seemingly to address a diagnostics/test failure involving HRESULT mismatches between implementations.
Changes:
- Removes legacy fallback behavior on exceptions in
IXCLRDataProcess.EnumMethodInstanceByAddress, returning the exception HRESULT instead. - Relaxes DEBUG-only HRESULT validation in
IXCLRDataMethodDefinition.EnumInstanceto allow cDAC success when the legacy DAC fails (AllowCdacSuccess).
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs | Changes exception handling for per-item method-instance enumeration (removes legacy fallback). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodDefinition.cs | Updates DEBUG HRESULT validation mode to tolerate cDAC success when legacy DAC fails. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 1
Comment on lines
1006
to
1009
| catch (System.Exception ex) | ||
| { | ||
| // The cDAC's IterateMethodInstances() implementation is incomplete compared | ||
| // to the native DAC's EnumMethodInstances::Next(). The native DAC uses a | ||
| // MethodIterator backed by AppDomain assembly iteration with EX_TRY/EX_CATCH | ||
| // error handling around each step. The cDAC re-implements this with | ||
| // IterateModules()/IterateMethodInstantiations()/IterateTypeParams() which | ||
| // call into IRuntimeTypeSystem and ILoader contracts. These contract calls | ||
| // (e.g. GetMethodTable, GetTypeHandle, GetMethodDescForSlot, GetModule, | ||
| // GetTypeDefToken) can throw when encountering method descs or type handles | ||
| // from assemblies/modules that the cDAC cannot fully process. This has been | ||
| // observed for generic method instantiations (cases 2-4 in | ||
| // IterateMethodInstances) in the SOS.WebApp3 integration test. | ||
| // | ||
| // Fall back to the legacy DAC result when available, otherwise propagate the error. | ||
| if (_legacyProcess is not null) | ||
| { | ||
| hr = hrLocal; | ||
| method.Interface = legacyMethod; | ||
| } | ||
| else | ||
| { | ||
| hr = ex.HResult; | ||
| } | ||
| hr = ex.HResult; | ||
| } |
noahfalk
reviewed
Aug 4, 2026
| { | ||
| hr = ex.HResult; | ||
| } | ||
| hr = ex.HResult; |
Member
There was a problem hiding this comment.
Does this mean that the underlying issue is already resolved and SOS.WebApp3 no longer fails in situations where the legacy DAC would have succeeded?
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.