Visitar URL original
fix: Stop shared values on plain DPs retaining their owners (backport #26123) by mergify[bot] · Pull Request #26142 · unoplatform/uno · GitHub
Skip to content

fix: Stop shared values on plain DPs retaining their owners (backport #26123) - #26142

Merged
Xiaoy312 merged 8 commits into
servicing/6.8from
mergify/bp/servicing/6.8/pr-26123
Nov 8, 2026
Merged

Xiaoy312 merged 8 commits into
servicing/6.8from
mergify/bp/servicing/6.8/pr-26123

Conversation

@mergify

@mergify mergify Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Note

Backport notes

  • Replaced Mergify's cherry-pick. 6.x keeps this code in DependencyObjectStore*.cs, where DependencyObject is still an interface, so master's commits don't apply as is. The automatic pick also added all of master's Given_Image_Release.cs, a file that 6.x doesn't have. This branch now carries:
    • the reporter's own servicing/6.8 patch as the first commit, by Jacques Bourque, kept as written. It also adds a ResourceDictionary-item guard on the bindable-child path, which master already has in ApplyDataContext.
    • the same follow-up commits, ported by hand: the holder count lives on the store, the weak owner link uses IWeakReferenceProvider.WeakReference (as 6.x's templated-parent link does), the tests are under Tests/Windows_UI_Xaml/, and the sample is under SamplesApp.Samples/Windows_UI_Xaml/Resources/.
  • The commit SHAs below are master's. The commits here have the same subjects, in the same order.
  • Validation (Skia desktop, SamplesApp.Skia.Generic):
    • The 15 new tests pass. The 3 two-owner tests fail with the reporter's 6.8 patch alone, as on master.
    • Regression set from the master PR: 898 of 899 pass. The one failure, Given_FrameworkElement_And_Leak.When_Add_Remove(CommandBarFlyout_Leak) (refcount 90 > 13), fails the same way on servicing/6.8 without this PR.
    • The reporter's repro on Uno.Sdk 6.8.0-dev.87: scenarios B, C, D and E leak without this PR, and nothing is retained with it.
    • WASM and mobile are covered by CI only.
  • The WinUI differences and the known Brush gap described below apply here as well.

GitHub Issue: closes unoplatform/kahua-private#570

PR Type:

🐞 Bugfix

What changed? 🚀

A shared value from a ResourceDictionary, such as a {StaticResource} from App.xaml, can be set on a custom dependency property registered with plain PropertyMetadata, for example from a control Style setter. Uno then tracks the value as a "bindable child" of the control. On master, a shared Style or Brush used this way keeps its first owner control, and that control's DataContext, alive for as long as the dictionary lives. For App.xaml, that is the app's lifetime.

The first commit is the reporter's patch, kept as written. Every change on top of it is a separate commit.

The reporter's fix (781c253)

  1. Weak owner link (root-cause fix). A shareable value (Style, Brush, Transition, …) recorded its owner in a strong _associatedParent field. That field was cleared only when the property was reassigned or a second owner appeared, so a single owner was rooted. The link is now weak, like the mentor link WinUI uses. A collected owner counts as no owner.
  2. Templates and styles take no DataContext. WinUI's FrameworkTemplate and Style have no DataContext. Their stores now opt out of inheritance context when they are constructed, so no path can cache a DataContext on them. They are also no longer tracked as bindable children, matching the existing exclusion on the ValueDoesNotInheritDataContext path.
  3. Release cleared and replaced values. Setting the property to null, a UIElement or a non-DependencyObject value now releases the previous child. The DataContext the owner pushed into it is cleared.

Follow-ups from review

  • Releasing a shared child broke the other holders (9965ecb). Item 3 cleared a released child's DataContext even when another owner, or another property of the same owner, still held the child with the same DataContext. That holder's bindings went blank. This regression came with item 3. Each child now counts the bindable slots holding it, and its DataContext is cleared only when the last one lets it go.
    • Cost: one int field per DependencyObject.
    • The count isn't decremented when an owner is collected while it still holds the child. In that case, the child's DataContext is not cleared on a later release, as before this PR.
  • No allocation per association (62d8ca9). The weak link allocated a finalizable WeakReference each time a shareable value got an owner, for example on every visual-state brush swap. It now borrows the owner's pooled SelfWeakReference, as the templated-parent link does.
  • ALC unload sweep (ffb0d3d, defensive hardening). If an owner was collected before its AssemblyLoadContext unloaded, the sweep saw no parent and kept the DataContext that owner had pushed. A collected owner now counts as a cleared association, as the strong reference used to guarantee.
  • Tests, comments that still described the strong field, and a catch in the sample's async void handler.

Behavior differences with WinUI

I checked the new tests on the native WinUI head in a local throwaway run, with the #if HAS_UNO guards removed. 11 of 15 pass there. These match WinUI:

  • the leak tests for a shared Style, Brush and DataTemplate, inside or outside a dictionary;
  • a template held in a collection;
  • the binding and ContentTemplate regression guards;
  • the two-owner tests: a child released by one holder keeps the DataContext of the remaining holder.

Uno is stricter than WinUI on purpose in two cases, because both keep the view model from being retained:

  • WinUI keeps a released child following its former owner's DataContext. Uno detaches it.
  • WinUI keeps the view model alive through a released child's binding while the child is alive. Uno clears it.

One test fails on WinUI because of how it is written: WinUI can't apply a Style that targets a control defined only in C#.

As in the reporter's patch, both test files stay #if HAS_UNO.

Known gap

A shared Brush held outside any ResourceDictionary (a static field or a code cache) is still left with its removed owner's view model. The owner itself is now collected. On master, both leaked. Dictionary values are covered by the existing IsResourceDictionaryItem guard, and templates and styles by item 2. Clearing that cached DataContext on unload would also need a re-push on reload, so it is tracked separately.

Validation

  • Runtime, Skia desktop (Win32):
    • The reporter's 8 tests (Given_SharedTemplate_DataContext) and 7 new ones (Given_PlainDP_SharedValue) pass.
    • Without the fix, 5 of the reporter's 8 fail on master, which matches the issue. A Style or DataTemplate held outside a dictionary also leaks on master (checked with equivalent probe tests).
    • The 3 two-owner tests fail with the reporter's commit alone and pass with the follow-up. The same scenario, as a probe test, passes on master.
    • Regression run: 1008 passed, 0 failed, 5 skipped, 1 inconclusive (WebGPU). It covers Given_FrameworkTemplate, Given_Style, Given_UIElement, Given_ContentControl, Given_ResourceDictionary, Given_DependencyObjectCollection, Given_CollectibleAlcAssociations, Given_Image_Release, Given_BindingMemoryLeak, the template and DataContext suites, Given_xBind, and the Windows_UI_Xaml_Data and Windows_UI_Xaml_Media namespaces.
  • Runtime, native WinUI (WinAppSDK): the local throwaway run described above.
  • Unit tests: Uno.UI.UnitTests has no new failures. The 13 Given_Calendar failures are the existing time-zone baseline.
  • Reporter's repro, against this build: the shared Style and shared Brush scenarios go from leaking the control and its view model on 7.1.0-dev.21 to nothing retained.
  • Compile: a strict desktop build (analyzers on) and dotnet format whitespace are clean.
  • Not run locally: WASM and mobile; CI will cover them.

The fix also applies to release/stable/6.7 and servicing/6.8. Those branches keep this code in DependencyObjectStore*.cs, so the backports will need manual ports.

PR Checklist ✅

🤖 Generated with Claude Code


This is an automatic backport of pull request #26123 done by [Mergify](https://mergify.com).

@mergify mergify Bot added the conflicts Automated merge hit conflicts label Nov 6, 2026
@mergify

This comment has been minimized.

@github-actions github-actions Bot added the area/automation Categorizes an issue or PR as relevant to project automation label Nov 6, 2026
jbourque-kahua and others added 8 commits November 6, 2026 10:49
Values held by a plain custom DependencyProperty (no FrameworkPropertyMetadataOptions) are tracked as "bindable children" of the owner. Several gaps let a shared value (e.g. a {StaticResource} from an app-lifetime ResourceDictionary) keep a removed owner, or its DataContext, alive:

- AssociateParent stored the owner of a shareable value (Style, Brush, Transition, ...) in a strong field, so a shareable value with a single owner rooted that owner (and its DataContext) until a second owner appeared. The association is now a weak reference, like the mentor link; a collected owner counts as no owner.
- FrameworkTemplate and Style have no DataContext in WinUI, but their stores accepted an inherited DataContext (a shared template cached the last owner's DataContext). They now opt out of inheritance context at construction, which covers every path (bindable child, collection item, parent propagation). They are also no longer tracked as bindable children, mirroring the existing exclusion on the ValueDoesNotInheritDataContext (mentor) path.
- A shareable value stored in a ResourceDictionary no longer caches an owner's DataContext through the bindable-child path, matching the existing ResourceDictionary guard on the parent-propagation path (and master's ApplyDataContext guard).
- Setting a plain DP to null, a UIElement or a non-DependencyObject value did not release the previous bindable child, which kept receiving the owner's DataContext (and kept its association). It is now released.
- A released child kept the DataContext the owner had pushed into it. It is now cleared, unless the child has its own parent or already holds a different DataContext.

Adds runtime tests (Given_SharedTemplate_DataContext: shared DataTemplate, Style and Brush on plain DPs, a template in a collection, released/cleared children, plus regression guards for a {Binding} on a plain DependencyObject value and for ContentControl.ContentTemplate) and a SamplesApp repro (Resources > SharedTemplateDataContextLeak). The weak association also fixes the existing Given_UIElement ContextFlyout/SelectionFlyout leak tests on this branch.

Targets servicing/6.8. A separate master port carries the same change (master already blocks DataContext caching on ResourceDictionary items).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A ResourceDictionary item never caches a DataContext, so the tests that
keep their shared value in a dictionary only prove the owner is released.
Add a Style and a DataTemplate held outside any dictionary (both leak the
view model without the template/style opt-out), and the issue's main
shape: an owner Style setter assigning a shared template.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A child held by two plain DPs (on two owners, or on two properties of one
owner) loses the DataContext the remaining holder pushes when one of them
releases it, so its bindings go blank.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Releasing a plain-DP child cleared the DataContext it had received
whenever it matched the releasing owner's, even when another owner (or
another property of the same owner) still held the child and pushed the
same DataContext, so that holder's bindings went blank. Count the
bindable slots holding a child and clear only when the last one lets go.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The weak owner association allocated a finalizable WeakReference (and a
GC handle) every time a shareable value got an owner, e.g. on every
visual-state brush swap. Reuse the owner's pooled self-reference
(IWeakReferenceProvider.WeakReference), as the templated-parent link
already does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the weak owner association, a parent collected before its ALC's
unload sweep read as "no parent", so the sweep left the DataContext that
parent had pushed into the shared value (only a DataContext whose own
type is collectible was still dropped). Treat a collected parent as a
cleared association, as the strong reference used to guarantee.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comments still named the removed _associatedParent field and described
the association as a strong reference. Shorten the bindable-child
exclusion comment, and have the mentor path use the same
template/Style predicate as the bindable-child path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
OnRun is async void and runs on Loaded, so an exception crashed the
SamplesApp instead of showing a result.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Xiaoy312
Xiaoy312 force-pushed the mergify/bp/servicing/6.8/pr-26123 branch from 17d9dbc to 7d498e9 Compare November 6, 2026 15:55
@unodevops

Copy link
Copy Markdown
Contributor

🤖 Your WebAssembly Sample App stage site is ready! Visit it here:

@Xiaoy312
Xiaoy312 merged commit 4a1ba1e into servicing/6.8 Nov 8, 2026
43 checks passed
@Xiaoy312
Xiaoy312 deleted the mergify/bp/servicing/6.8/pr-26123 branch November 8, 2026 03:44
mikernet added a commit to mikernet/uno that referenced this pull request Nov 13, 2026
The expired-slot step in AssociateParent (previous commit) and the
guarded release clear in UpdateChildBindable (unoplatform#26142) compare a value's
DataContext with its owner's, read with GetValue(DependencyProperty).
That overload checks the property's owner when
ValidatePropertyOwnerOnReadWrite is on (Debug builds, or any build with a
debugger attached). A POCO owner such as TextHighlighter keeps its values
in an attached store, whose DataContext property is owned by
AttachedDependencyObject while the store's type is the POCO's, so the
check threw "The Dependency Property [DataContext] is owned by
[AttachedDependencyObject] and cannot be used on [TextHighlighter]".
TextBlock hits the first one on every draw with a selection: it creates
a TextHighlighter for its selection brush, whose previous highlighter
has been collected. The second one fires when a highlighter's brush is
replaced. All these reads now go through the stores' DataContext
details, as SetMentoredChild and RepropagateMentoredChildrenDataContext
already do.
mikernet added a commit to Singulink/uno that referenced this pull request Nov 14, 2026
Takes Uno's servicing/6.8 from 6951c2c up to 5254ace (42 pull
requests). Conflicts, by area:

IME and TextBox (unoplatform#25984, backport of unoplatform#25804; unoplatform#25952, backport of unoplatform#25619;
unoplatform#25976, backport of unoplatform#25818)
- Takes upstream's composition design (unoplatform#25804): the browser's input is
  the source of truth for the text, compositions included. This drops
  the composition parts of the unoplatform#24502 port (90746b8): its
  TypeScript composition handling, the StartIndex/TextChangePending
  event arguments, the IsReadOnly forwarding to the hidden input, and
  its Given_TextBox.BrowserSoftKeyboard tests.
- Keeps the non-IME browser fixes on top of it, in the form their
  master pull requests have on unoplatform#25804: Enter raised once per press and
  tracked by key code (unoplatform#25174, with Given_TextBox.BrowserEnterKey from
  master in place of the Enter tests that lived in the dropped file),
  the iOS refocus on tap (unoplatform#25185), pastes without text and isIOS from
  BrowserInputHelper.
- Moves the unoplatform#26005 MaxLength fixes onto unoplatform#25804's native input path, as
  master has them: a refused browser edit keeps the TextBox's selection,
  TextBoxView.UpdateTextFromNative reports whether the edit went in, and
  the IME keeps its range on the applied text (master be85699, which
  d226897 had ported onto the unoplatform#24502 IME). Adds that commit's browser
  and platform-applied IME tests.
- TextBox.pointers.skia.cs: upstream's file (unoplatform#25619 iOS touch editing,
  unoplatform#25818, unoplatform#25804), with the unoplatform#25126 port (the hold acts when it starts,
  a touch right-tap keeps the caret) in the form master's unoplatform#25126 branch
  has it on unoplatform#25619, including focusing before the iOS hold timer acts
  and its iOS not-focusable test.

Pointers (unoplatform#25976, backport of unoplatform#25818)
- Tapped and RightTapped are raised once the PointerReleased bubbled
  (unoplatform#25818), with the unoplatform#25732 port's window coordinates (2-argument
  routed event args, RawPosition for ContextRequested), as master's
  unoplatform#25732 branch has them.
- The release wrapper of unoplatform#25818 keeps the unoplatform#25126 port's
  StopContextMenuTimer at the start of the release.

DependencyObject (unoplatform#26142, backport of unoplatform#26123)
- Takes upstream, then the code of unoplatform#25839 (97a2f17), which replaces
  this branch's unoplatform#25102 ports (800247f, be41da6, fd40bf9,
  7546379). DependencyObjectStore.Binder.cs, .alc.cs,
  Application.Alc.cs, ResourceDictionary.alc.cs and their tests now
  match unoplatform#25839; this drops 800247f's own
  SingleParentNonFE_Parent_Removes_Value_Then_Next_Parent_Starts_Clean
  test, which unoplatform#25839 doesn't have.

Imaging (unoplatform#25597, unoplatform#25915, unoplatform#25911, unoplatform#25897)
- Keeps this branch's side: it has the later forms of these fixes
  (unoplatform#25124, unoplatform#25559, unoplatform#25633 and unoplatform#26370 ports), where upstream has the
  6.8-only unoplatform#25597 design and earlier snapshots. Upstream's shared
  download, IgnoreImageCache and failed-load fixes are covered by
  SharedImageDecode. Adds upstream's When_Source_Outlives_Brushes test.

ToolTip (unoplatform#26098, backport of unoplatform#25137 and unoplatform#26096)
- Keeps this branch's unoplatform#25137 port and adds unoplatform#26096 in master's form: the
  service releases only a ToolTip it opened, and leaves one the app
  opened during the show delay to the app. Keeps both sides' tests.

Flyout (unoplatform#26062, backport of unoplatform#26060)
- This branch had the same commits already; keeps the "Keep
  non-resizable flyouts in window" port and its tests.

Also removes a helper pair in Given_AccessibleTextBox that both sides
added.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/automation Categorizes an issue or PR as relevant to project automation conflicts Automated merge hit conflicts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants