Conversation
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
...ve/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IContract.cs
Outdated
Show resolved
Hide resolved
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR introduces a Flush() mechanism across the cDAC contract system so cached contract data can be cleared when the target process state changes (e.g., after resume), and wires it into existing DAC flush entry points.
Changes:
- Add
Flush()toIContractandContractRegistry, and implement registry-wide flushing inCachingContractRegistry. - Invoke contract flushing from DAC-facing
Flush()/LockedFlush()implementations alongside_target.ProcessedData.Clear(). - Implement (mostly no-op)
Flush()methods across many contract implementations; clear internal dictionaries where applicable.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/CachingContractRegistry.cs | Implements registry-wide flushing by calling Flush() on cached contract instances. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs | Ensures IXCLRDataProcess.Flush clears both processed data and contract caches. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs | Updates ISOSDacInterface13.LockedFlush to clear processed data and contract caches. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Thread_1.cs | Adds contract Flush() (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/SyncBlock_1.cs | Adds contract Flush() (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/StackWalk_1.cs | Adds contract Flush() (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Signature/SignatureDecoder_1.cs | Clears cached signature provider dictionaries on Flush(). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/SHash_1.cs | Adds contract Flush() (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs | Clears RTS caches (method tables/descs/type handle caches) on Flush(). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeInfo_1.cs | Adds contract Flush() (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ReJIT_1.cs | Adds contract Flush() (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/PrecodeStubs_Common.cs | Adds contract Flush() (currently no-op) on common PrecodeStubs base. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/PlatformMetadata_1.cs | Adds contract Flush() (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.cs | Adds contract Flush() (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Notifications_1.cs | Adds contract Flush() (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs | Adds contract Flush() (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GCInfo/GCInfo_1.cs | Adds contract Flush() (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GC/GC_1.cs | Adds contract Flush() (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerCore.cs | Clears ExecutionManager code-info cache on Flush(). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManager_1.cs | Forwards Flush() to shared ExecutionManagerCore. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManager_2.cs | Forwards Flush() to shared ExecutionManagerCore. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Exception_1.cs | Adds contract Flush() (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/EcmaMetadata_1.cs | Clears cached metadata providers on Flush(). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/DebugInfo/DebugInfo_1.cs | Adds contract Flush() (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/DebugInfo/DebugInfo_2.cs | Adds contract Flush() (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/DacStreams_1.cs | Adds contract Flush() (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ComWrappers_1.cs | Adds contract Flush() (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CodeVersions_1.cs | Adds contract Flush() (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/BuiltInCOM_1.cs | Adds contract Flush() (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IContract.cs | Adds Flush() to the base contract interface. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs | Adds Flush() to the contract registry abstraction. |
...ve/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IContract.cs
Outdated
Show resolved
Hide resolved
...ative/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs
Show resolved
Hide resolved
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR introduces a unified “flush” pathway in cDAC to clear cached target/contract state when the debuggee’s state may have changed (e.g., resume), and wires that through existing legacy DAC COM flush entry points.
Changes:
- Add
Flush()toTarget,ContractRegistry, andIContract, and implement registry-wide flushing inCachingContractRegistry. - Implement per-contract cache clearing for several contracts (ExecutionManager, RuntimeTypeSystem, EcmaMetadata, SignatureDecoder).
- Route legacy DAC COM
Flush/LockedFlushcalls to_target.Flush().
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/tests/TestPlaceholderTarget.cs | Adds a ContractRegistry.Flush() override for the test registry (currently no-op). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/CachingContractRegistry.cs | Implements registry-wide flushing by calling Flush() on created contracts. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs | Routes IXCLRDataProcess.Flush() to _target.Flush(). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs | Routes ISOSDacInterface13.LockedFlush() to _target.Flush(). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Signature/SignatureDecoder_1.cs | Clears signature provider caches on flush. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs | Clears RuntimeTypeSystem dictionaries on flush. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerCore.cs | Clears code info cache on flush. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManager_1.cs | Forwards flush to core implementation. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManager_2.cs | Forwards flush to core implementation. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/EcmaMetadata_1.cs | Clears metadata cache on flush. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Target.cs | Adds Target.Flush() to clear ProcessedData and flush contracts. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IContract.cs | Adds IContract.Flush() default interface method. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs | Adds abstract ContractRegistry.Flush(). |
Comments suppressed due to low confidence (1)
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs:29
IXCLRDataProcess.Flushis a COM boundary and currently calls_target.Flush()without exception handling. With the newTarget.Flush()implementation calling intoContracts.Flush()(and ultimately per-contract cache clear logic), exceptions (e.g., from contract code) would escape across the COM boundary. Please wrap the body in a try/catch and returnex.HResult(or otherwise guaranteeTarget.Flush()is non-throwing).
int IXCLRDataProcess.Flush()
{
_target.Flush();
// As long as any part of cDAC falls back to the legacy DAC, we need to propagate the Flush call
if (_legacyProcess is not null)
return _legacyProcess.Flush();
return HResults.S_OK;
Complete Flush() impl by flushing contract caches along with all processed data.