Serialize DAC access and materialize unsafe enumerations#1473
Open
leculver wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a single DAC serialization model (DacLibrary.SyncRoot → SOSDac.SyncRoot) and updates key DAC-backed helpers to (1) serialize stateful DAC entry points and (2) materialize previously lazy enumerations before returning, preventing concurrent DAC enumerations from corrupting each other and avoiding caching of partial results.
Changes:
- Added a shared DAC
SyncRootand routed SOS/DAC helpers through it to serialize stateful enumerations and cache fills. - Converted multiple DAC-backed
IEnumerableimplementations from lazy iterators to fully materialized collections created under the DAC lock. - Updated thread/heap/runtime cache population to be lock-protected to prevent publishing truncated results; updated stack walking to enforce
maxFramesinside the DAC walk.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Diagnostics.Runtime/DacLibrary.cs | Introduces SyncRoot used as the central DAC serialization lock. |
| src/Microsoft.Diagnostics.Runtime/DacInterface/SosDac.cs | Exposes SyncRoot and adjusts internal name caches to be eagerly allocated dictionaries. |
| src/Microsoft.Diagnostics.Runtime/DacImplementation/DacTypeHelpers.cs | Serializes DAC calls and materializes method/field enumeration results under the lock. |
| src/Microsoft.Diagnostics.Runtime/DacImplementation/DacThreadHelpers.cs | Serializes stack root/trace enumeration and materializes results; enforces maxFrames inside the walk. |
| src/Microsoft.Diagnostics.Runtime/DacImplementation/DacServiceProvider.cs | Adds locking for singleton service construction and serializes heap initialization/flush against the DAC lock. |
| src/Microsoft.Diagnostics.Runtime/DacImplementation/DacRuntime.cs | Serializes/materalizes threads/appdomains/handles/jit manager enumerations. |
| src/Microsoft.Diagnostics.Runtime/DacImplementation/DacNativeHeaps.cs | Serializes and materializes native heap traversals and region enumerations. |
| src/Microsoft.Diagnostics.Runtime/DacImplementation/DacModuleHelpers.cs | Serializes module/metadata access; passes DAC lock into metadata reader. |
| src/Microsoft.Diagnostics.Runtime/DacImplementation/DacMethodLocator.cs | Serializes method lookup helpers through SyncRoot. |
| src/Microsoft.Diagnostics.Runtime/DacImplementation/DacMetadataReader.cs | Serializes metadata import calls and materializes enumerations under the DAC lock. |
| src/Microsoft.Diagnostics.Runtime/DacImplementation/DacLegacyThreadPool.cs | Serializes threadpool DAC reads through SyncRoot. |
| src/Microsoft.Diagnostics.Runtime/DacImplementation/DacHeap.cs | Serializes/materalizes heap-related enumerations and validates method tables under the DAC lock. |
| src/Microsoft.Diagnostics.Runtime/DacImplementation/DacComHelpers.cs | Serializes COM-related DAC reads and materializes RCW cleanup enumeration. |
| src/Microsoft.Diagnostics.Runtime/ClrThread.cs | Serializes stack root/trace cache population and updates stack trace helper signature usage. |
| src/Microsoft.Diagnostics.Runtime/ClrRuntime.cs | Serializes runtime cache population and adds handle caching with lock protection. |
| src/Microsoft.Diagnostics.Runtime/ClrHeap.cs | Serializes heap cache population for sync blocks, allocation contexts, and dependent handles. |
| src/Microsoft.Diagnostics.Runtime/AbstractDac/IAbstractThreadHelpers.cs | Updates stack trace helper contract to include maxFrames enforced by the DAC walk. |
Copilot's findings
- Files reviewed: 17/17 changed files
- Comments generated: 3
| } | ||
| } | ||
|
|
||
| return false; |
| } | ||
| } | ||
|
|
||
| return false; |
Comment on lines
+230
to
+231
| if (max-- == 0) | ||
| break; |
Introduce and apply the DacLibrary/SOS SyncRoot model to stateful DAC entry points. Serialize service construction, flush, stack walks, runtime/heap/thread cache population, and the remaining DAC helper surfaces so partial native enumerations cannot race and be permanently cached. Materialize native heap and other DAC traversal results under the lock, and enforce stack maxFrames inside the DAC walk before the helper can keep walking past the caller's cap.
leculver
force-pushed
the
split/dg-dac-serialization
branch
from
d43c438 to
c8355eb
Compare
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.
Summary
Introduces and applies a single DAC serialization model for stateful DAC entry points.
The DAC is not safe to call concurrently across independent helpers/enumerators. This PR adds and applies the
DacLibrary/SOSDacSyncRootconvention so DAC calls that can race native state are serialized and results are materialized before returning to callers.Included here:
SyncRootplumbing and documentationmaxFramesenforced inside the DAC walkSyncRootPart of #420.
Validation
dotnet build src/Microsoft.Diagnostics.Runtime/Microsoft.Diagnostics.Runtime.csproj --framework net10.0-- passed with 0 warnings and 0 errors.lockfree-mmf-threadsafe; all non-stress paths matched the original combined branch.