Fix FormattedString memory leak caused by shared Span by Dhivya-SF4094 · Pull Request #36294 · dotnet/maui

Dhivya-SF4094 · 2026-07-02T10:02:03Z

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Issue Details:

A shared, long-lived Span keeps a strong event reference to every FormattedString that uses it. As a result, even when those FormattedString instances are no longer used, the garbage collector cannot free them, causing a memory leak that grows over time.

Root Cause

Element.Parent already stores its parent as a weak reference, so the logical parent relationship (span.Parent = formattedString) was not causing the memory leak.
The actual leak came from the event subscriptions made when a Span was added to a FormattedString. In FormattedString.OnCollectionChanged, the FormattedString subscribed to the Span's PropertyChanged and PropertyChanging events.
These event handlers held strong references back to the FormattedString. As a result, if a Span was long-lived or shared, it kept every FormattedString it had been added to alive, preventing them from being garbage collected.

Description of Change

FormattedString.cs

Replaced the direct Span event subscriptions with per-occurrence weak subscription tokens.
Added two private nested types:
- SpanSubscription — represents a single subscription to one Span. It holds the owning FormattedString only through a WeakReference<FormattedString>, and subscribes to the span's PropertyChanging/PropertyChanged events. When an event fires, it forwards to the owner if still alive; if the owner has already been collected, it unsubscribes itself.
- SpanSubscriptions — manages a List<SpanSubscription> for the FormattedString. Each Add creates an independent subscription token, and Remove unsubscribes and removes a single matching token. This gives correct add/remove symmetry even when the same Span is added multiple times.
When a Span is added, a new SpanSubscription is created and tracked. When a Span is removed or the collection is cleared, the corresponding subscription is unsubscribed and removed from the list.
Because the WeakReference breaks the strong path from the span back to the FormattedString, a shared/long-lived Span no longer keeps discarded FormattedString instances alive.
Added a finalizer on SpanSubscriptions that unsubscribes any remaining subscriptions as a safety net, following the same cleanup pattern used by Border.

Issues Fixed:

Fixes #36289

Screenshots

Before	After

github-actions · 2026-07-02T10:02:11Z

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36294

Or

Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36294"

Dhivya-SF4094 · 2026-07-08T05:59:32Z

#36294 (review)

Verified the AI summary and incorporated the necessary updates.

Copilot

Pull request overview

This PR addresses a memory leak in FormattedString caused by strong event subscriptions to Span.PropertyChanged/PropertyChanging when a Span instance is shared or long-lived. The fix replaces direct event subscriptions with per-occurrence weak subscription tokens so shared spans no longer keep discarded FormattedString instances alive.

Changes:

Replace direct Span event subscriptions in FormattedString with per-occurrence weak subscription tokens (and cleanup logic).
Add unit tests covering duplicate-span add/remove symmetry and PropertyChanging behavior.
Add a memory-focused unit test that verifies a FormattedString can be garbage collected even when it contains a shared Span.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/Controls/src/Core/FormattedString.cs	Introduces per-occurrence weak subscription tokens for span property-change events to prevent shared-span retention of `FormattedString`.
src/Controls/tests/Core.UnitTests/FormattedStringTests.cs	Adds tests for duplicate-span subscription symmetry, `PropertyChanging` behavior, and a GC regression test for the shared-span leak.

+		sealed class SpanSubscriptions
+		{
+			readonly WeakReference<FormattedString> _owner;
+			readonly List<SpanSubscription> _subscriptions = new();
+
+			public SpanSubscriptions(FormattedString owner) => _owner = new(owner);
+
+			~SpanSubscriptions() => Clear();
+
+			public void Add(Span span) => _subscriptions.Add(new SpanSubscription(_owner, span));


MauiBot

Expert Review — 1 findings

See inline comments for details.

MauiBot · 2026-07-14T21:54:25Z

 						bo.Parent?.RemoveLogicalChild(bo);
-						bo.PropertyChanging -= OnItemPropertyChanging;
-						bo.PropertyChanged -= OnItemPropertyChanged;
+						_spanSubscriptions.Remove(bo);


🔍 AI-Generated Review (multi-model)

[major] Logic and Correctness — This now removes one event subscription per removed Span, but the logical-child cleanup in the same branch still removes through bo.Parent. For duplicate occurrences, the first removal clears span.Parent, so the second removal reaches this subscription cleanup while leaving the remaining logical child in this FormattedString; for a Span shared by two FormattedString instances, removing it from the first removes the logical child from the second instead. Please remove the occurrence from this FormattedString's logical children (and add a regression assertion) so subscription cleanup and logical-tree cleanup stay symmetric.

MauiBot

AI Review Summary

@Dhivya-SF4094 — new AI review results are available based on this last commit: a76e6ed. To request a fresh review after new comments or commits, comment /review rerun.

🗂️ Review Sessions — click to expand

🚦 Gate — Test Before & After Fix

Gate Result: ✅ PASSED

Platform: ANDROID · Base: main · Merge base: 0395a53b

Test	Without Fix (expect FAIL)	With Fix (expect PASS)
🧪 FormattedStringTests `FormattedStringTests`	✅ FAIL — 109s	✅ PASS — 83s

🔴 Without fix — 🧪 FormattedStringTests: FAIL ✅ · 109s

  Determining projects to restore...
  Restored /home/vsts/work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 878 ms).
  Restored /home/vsts/work/1/s/src/TestUtils/src/TestUtils/TestUtils.csproj (in 2.81 sec).
  Restored /home/vsts/work/1/s/src/Essentials/src/Essentials.csproj (in 4.49 sec).
  Restored /home/vsts/work/1/s/src/Core/src/Core.csproj (in 5.09 sec).
  Restored /home/vsts/work/1/s/src/Core/maps/src/Maps.csproj (in 2.58 sec).
  Restored /home/vsts/work/1/s/src/Controls/src/Xaml/Controls.Xaml.csproj (in 60 ms).
  Restored /home/vsts/work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 34 ms).
  Restored /home/vsts/work/1/s/src/Controls/Maps/src/Controls.Maps.csproj (in 42 ms).
  Restored /home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj (in 1.42 sec).
  1 of 10 projects are up-to-date for restore.
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14656892
  Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14656892
  Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14656892
  Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14656892
  Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net10.0/Microsoft.Maui.Maps.dll
  Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14656892
  Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14656892
  Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0/Microsoft.Maui.Controls.Xaml.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14656892
  Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0/Microsoft.Maui.Controls.Maps.dll
  TestUtils -> /home/vsts/work/1/s/artifacts/bin/TestUtils/Debug/netstandard2.0/Microsoft.Maui.TestUtils.dll
  Controls.Core.UnitTests -> /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (x64)

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.17]   Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.46]   Discovered:  Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.48]   Starting:    Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:02.50]     FormattedStringDoesNotLeak [FAIL]
[xUnit.net 00:00:02.50]       FormattedString should not be alive!
[xUnit.net 00:00:02.51]       Stack Trace:
[xUnit.net 00:00:02.51]         /_/src/Controls/tests/Core.UnitTests/FormattedStringTests.cs(208,0): at Microsoft.Maui.Controls.Core.UnitTests.FormattedStringTests.FormattedStringDoesNotLeak()
[xUnit.net 00:00:02.51]         --- End of stack trace from previous location ---
  Passed NullSpansNotAllowed [18 ms]
  Passed SpanChangesUnsubscribes [3 ms]
  Passed AddingSpanTriggersSpansPropertyChange [< 1 ms]
  Passed ImplicitStringConversion [4 ms]
  Passed SpanChangingTriggersSpansPropertyChanging [< 1 ms]
  Passed DuplicateSpanKeepsOneSubscriptionAfterSingleRemove [< 1 ms]
  Passed SpanChangeTriggersSpansPropertyChange [< 1 ms]
  Passed DuplicateSpanChangesUnsubscribes [< 1 ms]
  Failed FormattedStringDoesNotLeak [916 ms]
  Error Message:
   FormattedString should not be alive!
  Stack Trace:
     at Microsoft.Maui.Controls.Core.UnitTests.FormattedStringTests.FormattedStringDoesNotLeak() in /_/src/Controls/tests/Core.UnitTests/FormattedStringTests.cs:line 208
--- End of stack trace from previous location ---
[xUnit.net 00:00:02.53]   Finished:    Microsoft.Maui.Controls.Core.UnitTests
  Passed ImplicitStringConversionNull [< 1 ms]
  Passed SpanChangingUnsubscribesAfterRemoval [< 1 ms]

Test Run Failed.
Total tests: 11
     Passed: 10
     Failed: 1
 Total time: 3.0675 Seconds

🟢 With fix — 🧪 FormattedStringTests: PASS ✅ · 83s

  Determining projects to restore...
  All projects are up-to-date for restore.
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14656892
  Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14656892
  Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14656892
  Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14656892
  Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net10.0/Microsoft.Maui.Maps.dll
  Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14656892
  Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14656892
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14656892
  Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0/Microsoft.Maui.Controls.Maps.dll
  Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0/Microsoft.Maui.Controls.Xaml.dll
  TestUtils -> /home/vsts/work/1/s/artifacts/bin/TestUtils/Debug/netstandard2.0/Microsoft.Maui.TestUtils.dll
  Controls.Core.UnitTests -> /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (x64)

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.15]   Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.17]   Discovered:  Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.19]   Starting:    Microsoft.Maui.Controls.Core.UnitTests
  Passed NullSpansNotAllowed [12 ms]
  Passed SpanChangesUnsubscribes [4 ms]
  Passed AddingSpanTriggersSpansPropertyChange [< 1 ms]
  Passed ImplicitStringConversion [3 ms]
  Passed SpanChangingTriggersSpansPropertyChanging [< 1 ms]
  Passed DuplicateSpanKeepsOneSubscriptionAfterSingleRemove [< 1 ms]
  Passed SpanChangeTriggersSpansPropertyChange [< 1 ms]
  Passed DuplicateSpanChangesUnsubscribes [< 1 ms]
  Passed FormattedStringDoesNotLeak [28 ms]
[xUnit.net 00:00:01.34]   Finished:    Microsoft.Maui.Controls.Core.UnitTests
  Passed ImplicitStringConversionNull [< 1 ms]
  Passed SpanChangingUnsubscribesAfterRemoval [1 ms]

Test Run Successful.
Total tests: 11
     Passed: 11
 Total time: 1.8909 Seconds

📁 Fix files reverted (1 files)

src/Controls/src/Core/FormattedString.cs

📋 Pre-Flight — Context & Validation

Issue: Unknown - GitHub metadata unavailable (gh unauthenticated)
PR: #36294 - Local squashed PR branch over origin/main
Platforms Affected: android requested for testing; implementation is platform-neutral Controls core
Files Changed: 1 implementation, 1 test

Key Findings

Local PR diff changes src/Controls/src/Core/FormattedString.cs to prevent long-lived/shared Span instances from retaining FormattedString through PropertyChanging/PropertyChanged subscriptions.
Local tests added in src/Controls/tests/Core.UnitTests/FormattedStringTests.cs cover duplicate span occurrences, PropertyChanging, unsubscription after removal, and memory collection.
GitHub PR description, linked issue, comments, prior reviews, and required checks could not be fetched because the GitHub CLI is unauthenticated in this environment.
Existing MAUI weak-event proxy infrastructure (WeakNotifyPropertyChangingProxy, WeakNotifyPropertyChangedProxy) is a meaningful alternative to the PR's custom token-wrapper implementation.

Code Review Summary

Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 0 | Suggestions: 1

Key code review findings:

ℹ src/Controls/src/Core/FormattedString.cs: Consider using existing MAUI weak-event proxy infrastructure instead of custom nested subscription tokens.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #36294	Per-occurrence weak subscription token wrapping `Span.PropertyChanging`/`PropertyChanged` with weak owner reference	✅ PASSED (Gate provided)	`src/Controls/src/Core/FormattedString.cs`, `src/Controls/tests/Core.UnitTests/FormattedStringTests.cs`	Original PR

🔬 Code Review — Deep Analysis

Code Review - PR #36294

Independent Assessment

What this changes: Replaces FormattedString's direct subscriptions to each Span's PropertyChanging and PropertyChanged events with per-occurrence weak subscription tokens. The PR also adds unit tests for duplicate span occurrences, PropertyChanging propagation/unsubscription, and collection of a FormattedString attached to a long-lived shared Span.
Inferred motivation: A shared or long-lived Span can strongly retain every FormattedString that subscribed with instance-method delegates, causing FormattedString leaks.

Reconciliation with PR Narrative

Author claims: GitHub PR metadata could not be fetched because gh is unauthenticated in this environment.
Agreement/disagreement: The local code diff and tests support the inferred leak fix. The test FormattedStringDoesNotLeak specifically proves the long-lived-span retention path.

Prior Review Reconciliation

No prior review surfaces could be queried because gh is unauthenticated. No prior error findings were available locally.

Blast Radius Assessment

Runs for all instances: Yes. Every FormattedString now allocates a SpanSubscriptions helper and every added Span uses a weak subscription token.
Startup impact: No direct startup path; behavior runs when FormattedString instances are constructed/modified.
Static/shared state: No new static state in the PR fix.

CI Status

Required-check result: undetermined
Classification: undetermined
Action taken: GitHub CLI is unauthenticated (gh auth login required), so CI checks could not be queried. Confidence capped at low for CI evidence, but gate result was provided externally as passed.

Findings

💡 Suggestion - Prefer existing weak-event proxy infrastructure

The PR's custom SpanSubscription token is reasonable, but MAUI already has WeakNotifyPropertyChangingProxy and WeakNotifyPropertyChangedProxy patterns in WeakEventProxy.cs. A proxy-based implementation may be easier to maintain and align with existing Controls infrastructure.

Failure-Mode Probing

Shared long-lived Span: The PR avoids retaining FormattedString because SpanSubscription only holds a weak owner reference.
Duplicate span occurrences: The PR preserves per-occurrence subscription behavior, so one removal leaves one active subscription and removing both removes both.
Removed span mutation: Explicit Remove unsubscribes one matching occurrence, so removed spans should not notify once all occurrences are removed.
Long-lived span never fires after owner collection: Small subscription tokens can remain until finalizer cleanup, but the owner FormattedString is no longer strongly retained.

Verdict: NEEDS_DISCUSSION

Confidence: low
Summary: The local code looks directionally correct and the provided gate result says tests pass, but remote PR narrative, prior reviews, and CI checks were unavailable due GitHub CLI authentication. Existing MAUI weak-event proxies provide a credible alternative worth testing against the same unit gate.

🛠️ Fix — Analysis & Comparison

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	maui-expert-reviewer + code-review loop	Reuse existing `WeakNotifyPropertyChangingProxy` and `WeakNotifyPropertyChangedProxy` per span occurrence	✅ PASS — 11/11 focused tests	1 file	Preserves duplicate occurrence semantics and aligns with MAUI weak-event infrastructure
2	maui-expert-reviewer + learned comparison	Reuse existing weak proxies but register once per distinct `Span` with occurrence counting	✅ PASS — 11/11 focused tests	1 file	Fewer subscriptions, but may change exact duplicate-notification count semantics
PR	PR #36294	Custom per-occurrence `SpanSubscription` token with weak owner reference	✅ PASSED (Gate provided)	2 files	Original PR; tests already proved fail-without-fix/pass-with-fix

Cross-Pollination / Expert Review

Model	Round	New Ideas?	Details
maui-expert-reviewer	1	Yes	Suggested existing weak-event proxy registrations, counted distinct-span registrations, and a static/shared span observer registry.
local test loop	2	No implemented new low-risk idea	Static/shared registry was not implemented because both proxy-based candidates passed and the registry adds static state, thread-safety risk, and broader blast radius without clear benefit for this localized leak.

Candidate Details

try-fix-1

Approach: Existing weak-event proxy per span occurrence.
Test result: ✅ PASS — FormattedStringTests filtered run passed 11 tests.
Diff: CustomAgentLogsTmp/PRState/36294/PRAgent/try-fix-1/fix.diff
Assessment: Best alternative. It is demonstrably more consistent with existing MAUI weak-event infrastructure while preserving the PR's per-occurrence behavior.

try-fix-2

Approach: Existing weak-event proxy per distinct span with occurrence counting.
Test result: ✅ PASS — FormattedStringTests filtered run passed 11 tests.
Diff: CustomAgentLogsTmp/PRState/36294/PRAgent/try-fix-2/fix.diff
Assessment: Technically viable but less clearly compatible because it may reduce duplicate notifications from one-per-occurrence to one-per-distinct-span.

Exhausted: Yes — low-risk alternatives were tested; the remaining shared-registry idea is meaningfully different but not preferable because it introduces static/shared state and a wider failure surface.
Selected Fix: Candidate #1 — uses established MAUI weak-event proxy infrastructure and preserves the PR's observable duplicate-span semantics. It is the best alternative to consider if maintainability/style is prioritized over the PR's smaller custom wrapper.

📝 Recommended PR Title & Description

Assessment: ✏️ Recommend updating — the current description accurately explains the weak subscription leak fix, but the winning pr-plus-reviewer candidate also changes logical-child cleanup and adds regression coverage that the current metadata does not mention.

Recommended title

FormattedString: Fix memory leak caused by shared Span

Recommended description

### Issue Details:
A shared, long-lived Span keeps a strong event reference to every FormattedString that uses it. As a result, even when those FormattedString instances are no longer used, the garbage collector cannot free them, causing a memory leak that grows over time.

### Root Cause

- Element.Parent already stores its parent as a weak reference, so the logical parent relationship (span.Parent = formattedString) was not causing the memory leak.
- The actual leak came from the event subscriptions made when a Span was added to a FormattedString. In FormattedString.OnCollectionChanged, the FormattedString subscribed to the Span's PropertyChanged and PropertyChanging events.
- These event handlers held strong references back to the FormattedString. As a result, if a Span was long-lived or shared, it kept every FormattedString it had been added to alive, preventing them from being garbage collected.
- The same removal path also needs to clean up logical children from the FormattedString being modified, not from the Span's current Parent, because duplicate/shared Span scenarios can make Parent point somewhere else or become null.

### Description of Change

**FormattedString.cs**

- Replaced the direct Span event subscriptions with per-occurrence weak subscription tokens.

- Added two private nested types:

  - SpanSubscription — represents a single subscription to one Span. It holds the owning FormattedString only through a WeakReference<FormattedString>, and subscribes to the span's PropertyChanging/PropertyChanged events. When an event fires, it forwards to the owner if still alive; if the owner has already been collected, it unsubscribes itself.

  - SpanSubscriptions — manages a List<SpanSubscription> for the FormattedString. Each Add creates an independent subscription token, and Remove unsubscribes and removes a single matching token. This gives correct add/remove symmetry even when the same Span is added multiple times.

- When a Span is added, a new SpanSubscription is created and tracked. When a Span is removed or the collection is cleared, the corresponding subscription is unsubscribed and removed from the list.

- Logical-child cleanup now removes the old Span occurrence from the current FormattedString instead of routing through span.Parent, so duplicate/shared Span removal does not leave stale logical children or remove from another FormattedString.

- Because the WeakReference breaks the strong path from the span back to the FormattedString, a shared/long-lived Span no longer keeps discarded FormattedString instances alive.

- Added a finalizer on SpanSubscriptions that unsubscribes any remaining subscriptions as a safety net, following the same cleanup pattern used by Border.

**FormattedStringTests.cs**

- Added regression coverage for duplicate Span subscription removal, preserving one active subscription after a single duplicate removal, PropertyChanging cleanup, and collection of a FormattedString that used a shared long-lived Span.
- Added logical-child regression coverage for duplicate Span removal and shared Span removal across two FormattedString instances.

### Issues Fixed:
Fixes #36289

### Platforms Tested

- [x] Android

### Screenshots
| Before | After |
|---------|--------|
| <img width="634" height="375" src="https://github.com/user-attachments/assets/400c456e-c8ae-4c07-8856-a7d9ad9cd470"> | <img width="634" height="375" src="https://github.com/user-attachments/assets/62c8f19a-4a2b-4272-81ef-2f96df036e16"> |

🏁 Report — Final Recommendation

Comparative Fix Report — PR #36294

Candidate ranking

Rank	Candidate	Regression result	Assessment
1	`pr-plus-reviewer`	✅ Passed expanded focused regression suite in sandbox: 13/13 `FormattedStringTests`	Best candidate. Keeps the PR's per-occurrence weak subscription behavior and applies the expert reviewer's logical-child cleanup fix (`RemoveLogicalChild(bo)` on this `FormattedString`) with targeted duplicate/shared `Span` assertions.
2	`try-fix-1`	✅ Passed original focused suite: 11/11; ⚠️ inherits reviewer-found logical-child bug	Good weak-event implementation because it reuses MAUI's existing `WeakNotifyPropertyChangingProxy`/`WeakNotifyPropertyChangedProxy` per span occurrence, but it does not address the expert reviewer's logical-child cleanup issue.
3	`pr`	✅ Gate passed; ⚠️ expert reviewer found a major logical-child cleanup bug	Correctly breaks the shared-span event-retention leak with custom weak subscription tokens and preserves duplicate occurrence subscription semantics, but it is not merge-ready as-is because duplicate/shared spans can leave stale logical children or remove from the wrong `FormattedString`.
4	`try-fix-2`	✅ Passed original focused suite: 11/11; ⚠️ inherits reviewer-found logical-child bug	Viable leak fix, but weaker than `try-fix-1` and `pr` because distinct-span occurrence counting may change duplicate notification behavior from one callback per occurrence to one callback per distinct span.

Comparison details

pr replaces direct event subscriptions with custom per-occurrence subscription tokens that hold only a WeakReference<FormattedString> back to the owner. This addresses the reported memory leak and the supplied gate result confirms the tests fail without the fix and pass with the fix. The expert reviewer found a separate correctness issue in the same removal path: logical-child cleanup still routes through bo.Parent, which is not occurrence-safe for duplicate/shared Span instances.

pr-plus-reviewer applies the expert feedback by removing the old span occurrence from the current FormattedString's logical children instead of whatever bo.Parent currently points to. This is the only candidate that addresses both the memory leak and the logical-tree cleanup symmetry issue. It also adds regression coverage for duplicate span removal and shared span removal across two FormattedString instances.

try-fix-1 is the best STEP 5a alternative because it reuses existing MAUI weak-event proxy infrastructure and preserves per-occurrence subscription semantics. However, its diff only changes the weak-event subscription implementation and leaves the same bo.Parent?.RemoveLogicalChild(bo) cleanup in place, so it does not incorporate the expert reviewer's required fix.

try-fix-2 also uses existing weak-event proxies, but tracks one subscription per distinct Span with an occurrence count. It passed the original focused tests, but it is less behavior-preserving because duplicate span changes may produce one notification per distinct span rather than one per occurrence. Like try-fix-1, it does not fix the logical-child cleanup issue.

Winner

Winner: pr-plus-reviewer

pr-plus-reviewer is the single best candidate because it preserves the raw PR's successful weak subscription fix while correcting the reviewer-confirmed logical-child asymmetry. The STEP 5a alternatives are useful implementation references, especially try-fix-1, but neither addresses the expert review finding; try-fix-2 additionally carries a duplicate-notification behavior risk.

🧭 Next Steps — review latest findings

No alternative fix was selected for this run. Review the session findings and CI results before merging.

> [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Issue Details: A shared, long-lived Span keeps a strong event reference to every FormattedString that uses it. As a result, even when those FormattedString instances are no longer used, the garbage collector cannot free them, causing a memory leak that grows over time. ### Root Cause - Element.Parent already stores its parent as a weak reference, so the logical parent relationship (span.Parent = formattedString) was not causing the memory leak. - The actual leak came from the event subscriptions made when a Span was added to a FormattedString. In FormattedString.OnCollectionChanged, the FormattedString subscribed to the Span's PropertyChanged and PropertyChanging events. - These event handlers held strong references back to the FormattedString. As a result, if a Span was long-lived or shared, it kept every FormattedString it had been added to alive, preventing them from being garbage collected. ### Description of Change **FormattedString.cs** - Replaced the direct `Span` event subscriptions with per-occurrence weak subscription tokens. - Added two private nested types: - `SpanSubscription` — represents a single subscription to one `Span`. It holds the owning `FormattedString` only through a `WeakReference<FormattedString>`, and subscribes to the span's `PropertyChanging`/`PropertyChanged` events. When an event fires, it forwards to the owner if still alive; if the owner has already been collected, it unsubscribes itself. - `SpanSubscriptions` — manages a `List<SpanSubscription>` for the `FormattedString`. Each `Add` creates an independent subscription token, and `Remove` unsubscribes and removes a single matching token. This gives correct add/remove symmetry even when the same `Span` is added multiple times. - When a `Span` is added, a new `SpanSubscription` is created and tracked. When a `Span` is removed or the collection is cleared, the corresponding subscription is unsubscribed and removed from the list. - Because the `WeakReference` breaks the strong path from the span back to the `FormattedString`, a shared/long-lived `Span` no longer keeps discarded `FormattedString` instances alive. - Added a finalizer on `SpanSubscriptions` that unsubscribes any remaining subscriptions as a safety net, following the same cleanup pattern used by [Border](https://github.com/dotnet/maui/blob/main/src/Controls/src/Core/Border/Border.cs#L29). ### Issues Fixed: Fixes #36289 ### Screenshots | Before | After | |---------|--------| | <img width="634" height="375" src="https://github.com/user-attachments/assets/400c456e-c8ae-4c07-8856-a7d9ad9cd470"> | <img width="634" height="375" src="https://github.com/user-attachments/assets/62c8f19a-4a2b-4272-81ef-2f96df036e16"> |

dotnet-policy-service Bot added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Jul 2, 2026

Dhivya-SF4094 temporarily deployed to gh-aw-agents July 2, 2026 10:02 — with GitHub Actions Inactive

Dhivya-SF4094 temporarily deployed to gh-aw-agents July 2, 2026 10:04 — with GitHub Actions Inactive

Dhivya-SF4094 temporarily deployed to gh-aw-agents July 2, 2026 10:06 — with GitHub Actions Inactive

github-actions Bot added the area-controls-label Label, Span label Jul 2, 2026

Dhivya-SF4094 temporarily deployed to gh-aw-agents July 2, 2026 10:06 — with GitHub Actions Inactive

NirmalKumarYuvaraj added the community ✨ Community Contribution label Jul 2, 2026

This comment has been minimized.

Sign in to view

github-actions Bot added the s/agent-review-in-progress AI review is currently running for this PR label Jul 5, 2026

MauiBot added s/agent-gate-passed AI verified tests catch the bug (fail without fix, pass with fix) s/agent-fix-win AI found a better alternative fix than the PR s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review) labels Jul 5, 2026

This comment was marked as outdated.

Sign in to view

MauiBot removed the s/agent-review-in-progress AI review is currently running for this PR label Jul 5, 2026

Dhivya-SF4094 added 2 commits July 8, 2026 11:19

Fixed FormattedStringDoesNotLeak

e60e4eb

Updated Unshipped.txt

886c885

Dhivya-SF4094 force-pushed the fix-FormattedStringDoesNotLeak branch from 9bc7961 to 886c885 Compare July 8, 2026 05:50

Addressed AI summary concern

efd1d14

Dhivya-SF4094 force-pushed the fix-FormattedStringDoesNotLeak branch from f29f0ed to efd1d14 Compare July 8, 2026 05:57

This comment has been minimized.

Sign in to view

github-actions Bot added the s/agent-review-in-progress AI review is currently running for this PR label Jul 8, 2026

MauiBot removed the s/agent-review-in-progress AI review is currently running for this PR label Jul 8, 2026

Added additional test case

a76e6ed

Dhivya-SF4094 changed the title ~~[WIP] Fix FormattedString memory leak caused by shared Span~~ Fix FormattedString memory leak caused by shared Span Jul 13, 2026

build-analysis Bot mentioned this pull request Jul 13, 2026

Checkout failure: "Git fetch failed with exit code 128" dotnet/arcade#9009

Open

2 tasks

sheiksyedm marked this pull request as ready for review July 13, 2026 11:11

Copilot AI review requested due to automatic review settings July 13, 2026 11:11

Copilot started reviewing on behalf of sheiksyedm July 13, 2026 11:11 View session

Copilot AI reviewed Jul 13, 2026

View reviewed changes

github-actions Bot mentioned this pull request Jul 14, 2026

[PR Review Queue] 2026-07-14 #36560

Closed

This comment has been minimized.

Sign in to view

github-actions Bot added the s/agent-review-in-progress AI review is currently running for this PR label Jul 14, 2026

MauiBot reviewed Jul 14, 2026

View reviewed changes

MauiBot added s/agent-fix-pr-picked AI could not beat the PR fix - PR is the best among all candidates and removed s/agent-fix-win AI found a better alternative fix than the PR labels Jul 14, 2026

MauiBot reviewed Jul 14, 2026

View reviewed changes

MauiBot removed the s/agent-review-in-progress AI review is currently running for this PR label Jul 14, 2026

This was referenced Jul 15, 2026

[PR Review Queue] 2026-07-15 #36583

Closed

[PR Review Queue] 2026-07-16 #36610

Closed

kubaflo changed the base branch from main to inflight/current July 16, 2026 21:23

kubaflo merged commit 41500e2 into dotnet:inflight/current Jul 16, 2026
29 of 32 checks passed

github-actions Bot added this to the .NET 10 SR10 milestone Jul 16, 2026

github-actions Bot mentioned this pull request Jul 17, 2026

[repo-status] Daily Repo Status — July 17, 2026 #36623

Closed

Navigation Menu

Uh oh!

Fix FormattedString memory leak caused by shared Span - #36294

Fix FormattedString memory leak caused by shared Span#36294
kubaflo merged 4 commits into
dotnet:inflight/currentfrom
Dhivya-SF4094:fix-FormattedStringDoesNotLeak

Dhivya-SF4094 commented Jul 2, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 2, 2026

Uh oh!

This comment has been minimized.

This comment was marked as outdated.

Uh oh!

Dhivya-SF4094 commented Jul 8, 2026

Uh oh!

This comment has been minimized.

Copilot AI left a comment

Uh oh!

This comment has been minimized.

MauiBot left a comment

Uh oh!

MauiBot Jul 14, 2026

Uh oh!

MauiBot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

Dhivya-SF4094 commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue Details:

Root Cause

Description of Change

Issues Fixed:

Screenshots

Uh oh!

github-actions Bot commented Jul 2, 2026

Uh oh!

This comment has been minimized.

This comment was marked as outdated.

Uh oh!

Dhivya-SF4094 commented Jul 8, 2026

Uh oh!

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

This comment has been minimized.

MauiBot left a comment

Choose a reason for hiding this comment

Expert Review — 1 findings

Uh oh!

MauiBot Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

MauiBot left a comment

Choose a reason for hiding this comment

AI Review Summary

Gate Result: ✅ PASSED

Key Findings

Code Review Summary

Fix Candidates

Code Review - PR #36294

Independent Assessment

Reconciliation with PR Narrative

Prior Review Reconciliation

Blast Radius Assessment

CI Status

Findings

💡 Suggestion - Prefer existing weak-event proxy infrastructure

Failure-Mode Probing

Verdict: NEEDS_DISCUSSION

Fix Candidates

Cross-Pollination / Expert Review

Candidate Details

try-fix-1

try-fix-2

Comparative Fix Report — PR #36294

Candidate ranking

Comparison details

Winner

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Dhivya-SF4094 commented Jul 2, 2026 •

edited

Loading