{{ message }}
fix(alc): stop static Type-keyed caches pinning collectible AssemblyLoadContexts#23630
Open
nickrandolph wants to merge 5 commits into
Open
fix(alc): stop static Type-keyed caches pinning collectible AssemblyLoadContexts#23630nickrandolph wants to merge 5 commits into
nickrandolph wants to merge 5 commits into
Conversation
Contributor
|
Tick the box to add this pull request to the merge queue (same as
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #23629 by replacing several static, Type-keyed caches with weak-keyed alternatives to avoid pinning collectible AssemblyLoadContext instances (e.g., plugin/preview hosting) for the lifetime of the process.
Changes:
- Replace strong
Dictionary<Type, …>/HashtableExouter maps withConditionalWeakTable<Type, …>for routed-event flags, default style caching, and DP registries. - Adjust DP name-to-property caching to avoid caching entries for types loaded from non-default (collectible) ALCs.
- Replace
ContentControl’s per-type “has default template” memoization with a weak-keyed cache.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…pin collectible types ContentControl.HasDefaultTemplate is a static, process-lifetime memoized Func<Type,bool> backed by a Dictionary<Type,bool>. It is queried with the type of every measured ContentControl, so once a control type from a collectible AssemblyLoadContext (plugin / preview hosting) is measured, the dictionary retains that type's RuntimeType -> LoaderAllocator and pins the entire ALC for the process lifetime. Dump analysis showed this single cache as the sole external root pinning previewed-app ALCs. Back the memoization with a ConditionalWeakTable<Type, StrongBox<bool>> instead: type identity is canonical so weak-keyed lookup is equivalent, but a collectible type is now collectable once otherwise unreferenced. General fix (not ALC-specific): removes a steady managed leak for any long-running host that measures transient control types. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… types Style._defaultStyleCache / _nativeDefaultStyleCache mapped Type -> resolved default Style in a process-lifetime Dictionary. Queried during rendering with the type of every styled control, they retained control types from collectible AssemblyLoadContexts (plugin / preview hosting) via RuntimeType -> LoaderAllocator, pinning the whole ALC. The existing ClearCachesForNonDefaultAlc eviction cannot win the race: a render after unload re-populates the entry. Back both caches with a ConditionalWeakTable<Type, StrongBox<Style?>> (weak identity keys; the StrongBox lets a null resolved style still be cached). Type identity is canonical so lookup is equivalent, but a collectible type is now collectable once otherwise unreferenced — structurally removing the pin regardless of render/unload ordering. The strongly-keyed provider lookups still use explicit eviction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The DependencyProperty registry (Type -> per-type property table) and the inherited- properties-for-type cache were strong HashtableEx maps keyed by owner Type. Registering / looking up a DependencyProperty for a control type from a collectible AssemblyLoadContext retained that type (Type -> RuntimeType -> LoaderAllocator), pinning the whole ALC; the existing RemoveNonDefaultAlcEntries eviction loses the race against the unloading app's residual DP activity re-populating the entry. Back the outer Type-keyed maps with ConditionalWeakTable<Type, ...> (weak identity keys). Type identity is canonical so lookup is equivalent; collectible types are now collectable once otherwise unreferenced, removing the pin structurally. Eviction is retained as eager cleanup. Inner per-type name tables stay strong and collect with their weak type key. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nt caches - NameToPropertyDictionary: skip caching entries whose owner type is from a collectible ALC (composite key can't be weak-keyed); they re-resolve via the weak-keyed registry. - UIElementGeneratedProxy: weak-key the Type -> RoutedEventFlag cache via ConditionalWeakTable (a miss must not be misread as no-routed-events, so keep caching with weak keys rather than skipping).
Addresses review feedback (#23629): avoid the null-forgiving operator by returning false for a null type instead of calling the non-nullable GetDefaultStyleForType with a suppressed null. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
nickrandolph
force-pushed
the
dev/nr/alc-weak-type-keyed-caches
branch
from
1cd1a4f to
91074fb
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.
ALC pin-fix stack — PR 2 of 4 (uno). Base:
dev/nr/alc-weak-associated-parent. Next: U3 (ItemsView DEBUG) stacks on this branch.Fixes #23629