Fix Static/ThreadStatic walking#1477
Open
leculver wants to merge 4 commits into
Open
Conversation
…oft#1474) On .NET 9+ Core the runtime stores non-collectible thread statics in a per-thread Object[] referenced by the native ThreadLocalData (pNonCollectibleTlsArrayData), scanned directly by the GC as part of the owning thread's roots -- not a GC handle and not on the stack. ClrHeap .EnumerateRoots only enumerated handles, finalizer roots, and stack roots, so an object rooted only by a [ThreadStatic] field had no enumerable root and GCRoot.EnumerateRootPaths returned nothing for it. Pre-.NET 9 these were folded into the handle table, so this is a .NET 10 regression. Add ClrRootKind.ThreadStaticVar and IAbstractTypeHelpers.EnumerateThreadStaticRoots(moduleAddress, threadAddress), gated to flavor==Core && CLR major>=9 (empty otherwise). The DAC implementation walks the module's constructed types via the type-def map (no ClrType/ClrField hydration), filters with metadata (skip types with no static FieldDef), and yields each type's non-zero GC thread-static base from ISOSDacInterface14.GetThreadStaticBaseAddress. Per-module candidate MethodTable sets are cached in a ConcurrentDictionary (thread-independent and safe for concurrent enumeration) so the walk is O(modules), not O(threads*modules); per-type DAC faults are contained so one corrupt type cannot abort root enumeration. ClrHeap exposes the new public EnumerateThreadStaticRoots() (also part of IClrHeap and included in EnumerateRoots), yielding each base as a ThreadStaticVar root; on non-Core or pre-9 targets it returns empty. Adds a [ThreadStatic]-only rooted object to the GCRoot test target and a ThreadStaticRootsAreEnumerated regression test.
…onalRoots (microsoft#1474) On .NET 9/10 a type's GC statics live in a shared pinned Object[] (the runtime's PinnedHeapHandleTable bucket) kept alive by a single strong handle. On Server GC with DATAS the DAC's handle walker can fail to enumerate that handle (DacHandleWalker::WalkHandles walks only GCHeapCount() per-proc handle slots, fewer than the actual slot count once the dynamic heap count is reduced), so an object reachable only through a regular static has no enumerable root and GCRoot.EnumerateRootPaths returns nothing. Verified on the issue's customer Server-GC dump: GCRoot went from 0 to 1 path via a StaticVar root through the exact bucket whose strong handle the DAC skipped. Replace the separate EnumerateThreadStaticRoots/EnumerateStaticRoots (neither shipped) with a single ClrHeap.EnumerateAdditionalRoots() backed by one tagged DAC walk, IAbstractTypeHelpers.EnumerateAdditionalRoots(module, threads) -> AdditionalRootInfo{Address, IsThreadStatic}. One module pass covers both static kinds; the cached per-module candidate MethodTable set is reused. Adds ClrRootKind.StaticVar; regular-static bases are interior to the pinned bucket and resolved to (and deduped by) their containing object, reported pinned. Scoped to Core major 9-10 (pre-9 folded statics into the handle table; .NET 11+ enumerates the handle table and statics correctly). Per-type DAC faults are contained so one corrupt type cannot abort root enumeration. Adds a regular-static-rooted object to the GCRoot test target and a StaticRootsAreEnumerated regression test; updates ThreadStaticRootsAreEnumerated to the unified API. Full GCRoot suite (38) green.
…with NotImplemented default Replace AdditionalRootInfo.IsThreadStatic with an extensible AdditionalRootKind enum (StaticVariable, ThreadStaticVariable) plus a separate IsInteriorPointer flag so the address semantics are decoupled from the root kind and future kinds can be added cleanly. ClrHeap.EnumerateAdditionalRoots now decides whether to resolve a containing object from IsInteriorPointer (honored for any kind, not just statics) and maps Kind -> ClrRootKind via a switch whose default throws NotImplementedException, so an unhandled future kind fails loudly. Add a last-resolved-object fast path for interior bases to avoid repeated GetContainingObject calls. A single last-object cache is insufficient on its own because the DAC interleaves bases across the (few) live static buckets rather than grouping them -- caching alone re-yielded buckets (186 vs 3 roots, 86 vs 1 path on the customer dump) -- so the fast path is backed by the small resolved-bucket set that still guarantees each bucket is reported exactly once. Verified on the customer dumps (faulty: 3 StaticVar roots, 1 GCRoot path) and full GCRoot suite (38) green.
Within EnumerateAdditionalRoots a heap object is either a static bucket or a thread-static storage object, never both, so there is no value in reporting the same object twice -- a single HashSet<ulong> of already-reported object addresses is sufficient (no need for per-kind sets). The multi-kind reporting of one object (e.g. a bucket seen as StrongHandle + Stack + StaticVar) happens across the separate enumerators composed by EnumerateRoots, not within this method. Dedup now lives entirely at the ClrHeap layer (robust to any IAbstractTypeHelpers output); the last-object cache and resolved-bucket range list are demoted to a pure perf cache that only avoids repeated GetContainingObject calls. Verified on the customer dumps (faulty: 3 StaticVar roots, 1 GCRoot path) and full GCRoot suite (38) green.
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
1 participant
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.
StaticVariables: The DATAS feature broke HandleTable walking. When we downscale the number of GC heaps, this affected the dac's view of the HandleTable, skipping the difference between the number of processors (which is how many subtables were created) and the current GC count. These used to always be in alignment but now are not.
ThreadStaticVariables: ThreadStatics are now handled completely separately from the rest of the code. We need to update that in the cDac in the runtime, but for now we are able to find the right values and enumerate them.
However, full handle table walking is broken, and likely can't be fixed until .Net 11 releases.
Fixes #1474.