Repository navigation
Fix for memory leak in Accelerometer.ReadingChanged/ShakeDetected by using weak event source - #36503
Conversation
…hanged/ShakeDetected by backing them with a weak event source instead of plain multicast delegates.
GitHub Actions
Failure
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36503Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36503" |
dotnet-policy-service
Bot
added
the
partner/syncfusion
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
added
the
s/agent-review-in-progress
MauiBot
added
s/agent-gate-passed
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
MauiBot
removed
the
s/agent-review-in-progress
kubaflo
left a comment
kubaflo
left a comment
There was a problem hiding this comment.
Could you please check the ai's suggestions?
…ith ConditionalWeakTable-backed direct delegate invocation in AccelerometerWeakEventSource, preserving original exception semantics and pruning dead subscriptions in HasHandlers.
I have applied the suggested try-fix-1 approach:
|
@kubaflo, I have addressed the AI review concerns and updated the code based on the suggestions. |
There was a problem hiding this comment.
Pull request overview
This PR hardens Microsoft.Maui.Devices.Sensors.Accelerometer against app-lifetime memory leaks by switching ReadingChanged/ShakeDetected from strong multicast delegate storage to a weak-subscriber event source, and adds unit tests to validate both collection and behavior parity (live subscribers + exception propagation).
Changes:
- Replace
AccelerometerImplementation.ReadingChanged/ShakeDetectedbacking storage with weak event sources. - Add
WeakEventSource/WeakEventSource<TEventArgs>implementation to hold instance handlers without strongly retaining subscriber targets. - Add unit tests covering: forgotten unsubscribe does not leak, live subscriber still receives events after GC, and subscriber exceptions propagate directly.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/Essentials/test/UnitTests/Accelerometer_Tests.cs | Adds tests for leak prevention, live-subscriber behavior after GC, and exception propagation semantics. |
| src/Essentials/src/Accelerometer/AccelerometerWeakEventSource.shared.cs | Introduces weak event source implementations for generic and non-generic EventHandler patterns. |
| src/Essentials/src/Accelerometer/Accelerometer.shared.cs | Reworks ReadingChanged/ShakeDetected event backing and dispatch to use weak event sources and HasHandlers gating. |
| /// <summary> | ||
| /// Gets whether there is at least one subscription. This is a plain count check and does | ||
| /// not prune dead subscriptions on every read (pruning happens lazily in <see cref="Raise"/>, | ||
| /// <see cref="Subscribe"/>, and <see cref="Unsubscribe"/>), so it is cheap to query on every | ||
| /// sensor reading without the cost of walking/mutating the subscription list each time. | ||
| /// </summary> |
| /// <summary> | ||
| /// Gets whether there is at least one subscription. This is a plain count check and does | ||
| /// not prune dead subscriptions on every read (pruning happens lazily in <see cref="Raise"/>, | ||
| /// <see cref="Subscribe"/>, and <see cref="Unsubscribe"/>), so it is cheap to query on every | ||
| /// sensor reading (e.g. to gate <c>ShakeDetected</c> processing) without the cost of | ||
| /// walking/mutating the subscription list each time. | ||
| /// </summary> |
| try | ||
| { | ||
| Accelerometer.Stop(); | ||
| } | ||
| catch | ||
| { | ||
| // Not implemented in the reference assembly used by unit tests; the leak | ||
| // reproduces independently of Start()/Stop() actually running. | ||
| } |
| internal void OnChanged(AccelerometerChangedEventArgs e) | ||
| { | ||
| if (useSyncContext) | ||
| MainThread.BeginInvokeOnMainThread(() => ReadingChanged?.Invoke(null, e)); | ||
| { | ||
| MainThread.BeginInvokeOnMainThread(() => readingChangedSource.Raise(null, e)); | ||
| } | ||
| else | ||
| ReadingChanged?.Invoke(null, e); | ||
| { | ||
| readingChangedSource.Raise(null, e); | ||
| } | ||
|
|
||
| if (ShakeDetected != null) | ||
| if (shakeDetectedSource.HasHandlers) | ||
| { | ||
| ProcessShakeEvent(e.Reading.Acceleration); | ||
| } |
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
added
the
s/agent-review-in-progress
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 1 findings
See inline comments for details.
|
|
||
| lock (gate) | ||
| { | ||
| var target = handler.Target; |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[major] Event Semantics / Weak Lifetime — Subscribe treats the incoming delegate as a single target/method, but callers can legally subscribe a multicast delegate (for example handler = a.OnReadingChanged; handler += b.OnReadingChanged; Accelerometer.ReadingChanged += handler). In that case handler.Target is only the last invocation-list target while the closed multicast delegate stored in the ConditionalWeakTable value strongly references the other targets for as long as the last target is alive, so earlier subscribers can still leak; if the last target dies first, live earlier handlers are silently dropped. Please split handler.GetInvocationList() and store each single-cast delegate independently (same issue in the non-generic Subscribe below).
MauiBot
added
s/agent-fix-pr-picked
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@BagavathiPerumal — new AI review results are available based on this last commit:
df61be2. 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) |
|---|---|---|
🧪 Accelerometer_Tests Accelerometer_Tests |
✅ FAIL — 20s | ✅ PASS — 9s |
🔴 Without fix — 🧪 Accelerometer_Tests: FAIL ✅ · 20s
Determining projects to restore...
Restored /home/vsts/work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 682 ms).
Restored /home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj (in 3.44 sec).
Restored /home/vsts/work/1/s/src/Essentials/src/Essentials.csproj (in 4.64 sec).
##vso[build.updatebuildnumber]10.0.100-ci+azdo.14638167
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.100-ci+azdo.14638167
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
Essentials.UnitTests -> /home/vsts/work/1/s/artifacts/bin/Essentials.UnitTests/Debug/net10.0/Microsoft.Maui.Essentials.UnitTests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Essentials.UnitTests/Debug/net10.0/Microsoft.Maui.Essentials.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.10] Discovering: Microsoft.Maui.Essentials.UnitTests
[xUnit.net 00:00:00.35] Discovered: Microsoft.Maui.Essentials.UnitTests
[xUnit.net 00:00:00.36] Starting: Microsoft.Maui.Essentials.UnitTests
Passed Tests.Accelerometer_Tests.Accelerometer_Comparison(x1: 0, y1: 0, z1: 0, x2: 0, y2: 0, z2: 1, equals: False) [13 ms]
Passed Tests.Accelerometer_Tests.Accelerometer_Comparison(x1: 0, y1: 0, z1: 0, x2: 0, y2: 1, z2: 0, equals: False) [< 1 ms]
Passed Tests.Accelerometer_Tests.Accelerometer_Comparison(x1: 0, y1: 0, z1: 0, x2: 1, y2: 0, z2: 0, equals: False) [< 1 ms]
Passed Tests.Accelerometer_Tests.Accelerometer_Comparison(x1: 0, y1: 0, z1: 0, x2: 0, y2: 0, z2: 0, equals: True) [6 ms]
Passed Tests.Accelerometer_Tests.ShakingTests [2 ms]
Passed Tests.Accelerometer_Tests.Accelerometer_IsSupported [1 ms]
Passed Tests.Accelerometer_Tests.ClearQueue [< 1 ms]
Passed Tests.Accelerometer_Tests.Accelerometer_ReadingChanged_PropagatesSubscriberExceptionDirectly [1 ms]
Passed Tests.Accelerometer_Tests.Accelerometer_ReadingChanged_StillNotifiesLiveSubscriberAfterGC [8 ms]
[xUnit.net 00:00:00.54] Tests.Accelerometer_Tests.Accelerometer_ReadingChanged_DoesNotLeakForgottenSubscriber [FAIL]
[xUnit.net 00:00:00.54] Accelerometer.ReadingChanged subscriber was still alive after GC; a forgotten unsubscribe should not leak the subscriber for the process lifetime.
[xUnit.net 00:00:00.54] Stack Trace:
[xUnit.net 00:00:00.54] /_/src/Essentials/test/UnitTests/Accelerometer_Tests.cs(25,0): at Tests.Accelerometer_Tests.Accelerometer_ReadingChanged_DoesNotLeakForgottenSubscriber()
[xUnit.net 00:00:00.54] at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
[xUnit.net 00:00:00.54] at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
[xUnit.net 00:00:00.55] Finished: Microsoft.Maui.Essentials.UnitTests
Passed Tests.Accelerometer_Tests.Accelerometer_IsMonitoring [< 1 ms]
Passed Tests.Accelerometer_Tests.InitialShaking [< 1 ms]
Passed Tests.Accelerometer_Tests.Accelerometer_Start [< 1 ms]
Passed Tests.Accelerometer_Tests.Accelerometer_Stop [2 ms]
Failed Tests.Accelerometer_Tests.Accelerometer_ReadingChanged_DoesNotLeakForgottenSubscriber [12 ms]
Error Message:
Accelerometer.ReadingChanged subscriber was still alive after GC; a forgotten unsubscribe should not leak the subscriber for the process lifetime.
Stack Trace:
at Tests.Accelerometer_Tests.Accelerometer_ReadingChanged_DoesNotLeakForgottenSubscriber() in /_/src/Essentials/test/UnitTests/Accelerometer_Tests.cs:line 25
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
Test Run Failed.
Total tests: 14
Passed: 13
Failed: 1
Total time: 1.3027 Seconds
🟢 With fix — 🧪 Accelerometer_Tests: PASS ✅ · 9s
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.100-ci+azdo.14638167
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.100-ci+azdo.14638167
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
Essentials.UnitTests -> /home/vsts/work/1/s/artifacts/bin/Essentials.UnitTests/Debug/net10.0/Microsoft.Maui.Essentials.UnitTests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Essentials.UnitTests/Debug/net10.0/Microsoft.Maui.Essentials.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.11] Discovering: Microsoft.Maui.Essentials.UnitTests
[xUnit.net 00:00:00.43] Discovered: Microsoft.Maui.Essentials.UnitTests
[xUnit.net 00:00:00.44] Starting: Microsoft.Maui.Essentials.UnitTests
Passed Tests.Accelerometer_Tests.Accelerometer_Comparison(x1: 0, y1: 0, z1: 0, x2: 0, y2: 0, z2: 1, equals: False) [13 ms]
Passed Tests.Accelerometer_Tests.Accelerometer_Comparison(x1: 0, y1: 0, z1: 0, x2: 0, y2: 1, z2: 0, equals: False) [< 1 ms]
Passed Tests.Accelerometer_Tests.Accelerometer_Comparison(x1: 0, y1: 0, z1: 0, x2: 1, y2: 0, z2: 0, equals: False) [< 1 ms]
Passed Tests.Accelerometer_Tests.Accelerometer_Comparison(x1: 0, y1: 0, z1: 0, x2: 0, y2: 0, z2: 0, equals: True) [8 ms]
Passed Tests.Accelerometer_Tests.ShakingTests [2 ms]
Passed Tests.Accelerometer_Tests.Accelerometer_IsSupported [2 ms]
Passed Tests.Accelerometer_Tests.ClearQueue [< 1 ms]
Passed Tests.Accelerometer_Tests.Accelerometer_ReadingChanged_PropagatesSubscriberExceptionDirectly [6 ms]
Passed Tests.Accelerometer_Tests.Accelerometer_ReadingChanged_StillNotifiesLiveSubscriberAfterGC [8 ms]
[xUnit.net 00:00:00.62] Finished: Microsoft.Maui.Essentials.UnitTests
Passed Tests.Accelerometer_Tests.Accelerometer_IsMonitoring [< 1 ms]
Passed Tests.Accelerometer_Tests.InitialShaking [< 1 ms]
Passed Tests.Accelerometer_Tests.Accelerometer_Start [< 1 ms]
Passed Tests.Accelerometer_Tests.Accelerometer_Stop [2 ms]
Passed Tests.Accelerometer_Tests.Accelerometer_ReadingChanged_DoesNotLeakForgottenSubscriber [5 ms]
Test Run Successful.
Total tests: 14
Passed: 14
Total time: 1.3987 Seconds
📁 Fix files reverted (1 files)
src/Essentials/src/Accelerometer/Accelerometer.shared.cs
New files (not reverted):
src/Essentials/src/Accelerometer/AccelerometerWeakEventSource.shared.cs
📱 UI Tests — Essentials
Detected UI test categories: Essentials
⏭️ Deep UI tests — 0 passed, 0 failed across 1 category on platform-pool agent (replaces in-process counts above). 1 category reported 0 tests.
🧪 UI Test Execution Results (deep, platform pool)
| Category | Tests | Snapshot diffs |
|---|---|---|
Essentials |
0 tests | — |
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs) |
📋 Pre-Flight — Context & Validation
Issue: #36216 - Essentials sensor static events (Accelerometer.ReadingChanged et al.) leak subscribers — no WeakEventManager, Stop() doesn't unsubscribe
PR: #36503 - Fix for memory leak in Accelerometer.ReadingChanged/ShakeDetected by using weak event source
Platforms Affected: Android requested for testing; implementation is shared Essentials sensor code affecting Android, iOS, Mac Catalyst, Windows, Tizen
Files Changed: 2 implementation, 1 test
Key Findings
Accelerometer.Defaultis an app-lifetime singleton, so strong event backing fields retain subscribers that forget-=for the process lifetime.- Current PR replaces strong event fields with custom weak event sources and adds unit coverage for forgotten-subscriber collection, live-subscriber delivery after GC, and direct subscriber exception propagation.
- The current PR already incorporates the prior direct-delegate/
ConditionalWeakTableapproach from earlier AI feedback. - Main review concern is not correctness of the leak fix, but tradeoffs: weak event lifetime semantics differ from normal .NET events, and the current source does per-reading locking/pruning/allocation work.
Code Review Summary
Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 2 | Suggestions: 1
Key code review findings:
- ⚠ Weak events can silently drop subscriptions whose delegate target is only rooted by the event.
- ⚠ Sensor hot path now allocates a handler array during
Raiseand locks/prunes duringHasHandlerschecks. - ℹ
HasHandlersXML comments should match actual pruning behavior/cost.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #36503 | Custom weak event source using weak subscriber targets plus ConditionalWeakTable-held closed delegates, invoked directly |
✅ PASSED (Gate supplied) | Accelerometer.shared.cs, AccelerometerWeakEventSource.shared.cs, Accelerometer_Tests.cs |
Original PR; gate result supplied by caller |
🔬 Code Review — Deep Analysis
Code Review — PR #36503
Independent Assessment
What this changes: Replaces AccelerometerImplementation’s strong multicast event fields with custom weak event sources for ReadingChanged and ShakeDetected, plus unit tests for leak prevention, live subscriber delivery after GC, and direct exception propagation.
Inferred motivation: Prevent Accelerometer.Default (an app-lifetime singleton) from retaining page/view-model subscribers forever when callers forget to unsubscribe.
Reconciliation with PR Narrative
Author claims: Fixes memory leaks in Accelerometer.ReadingChanged / ShakeDetected by weakly referencing subscribers; avoids WeakEventManager due project constraints and reflection exception wrapping.
Agreement/disagreement: The code matches the stated leak fix and avoids MethodInfo.Invoke. The main unresolved concern is that this intentionally changes ordinary .NET event lifetime semantics for public events.
Prior Review Reconciliation
| Prior ❌ Error Finding | Source | Status | Evidence |
|---|---|---|---|
Fix-MilestoneDrift.ps1 StrictMode crash |
MauiBot prior review | 🔄 Obsolete | Current PR file list contains only the three Accelerometer files; the .github/scripts files were dirty local state in a previous review, not this PR. |
Blast Radius Assessment
- Runs for all instances: Yes — affects all
Accelerometer.Defaultevent subscribers. - Startup impact: No direct startup path impact.
- Static/shared state: Yes — changes event storage on the static singleton implementation.
CI Status
- Required-check result: unavailable via authenticated
gh; public context and prior gate output show gate passed while current required checks were not conclusively classified locally. - Classification: undetermined / pending.
- Action taken: Confidence capped low; no LGTM.
Findings
❌ Error
No current ❌ Error findings.
⚠️ Warning — Weak events can silently drop still-logical subscriptions
src/Essentials/src/Accelerometer/AccelerometerWeakEventSource.shared.cs stores instance subscribers weakly. This prevents leaks, but changes normal event semantics: a subscription whose delegate target is only rooted by the event can now be collected and stop receiving readings. A common case is a lambda/closure created in a setup method that captures local state; after that method returns, the closure may be collected even though the caller expected the event subscription to keep it alive until -= or Stop().
⚠️ Warning — Sensor hot path now allocates and locks per reading
WeakEventSource<TEventArgs>.Raise allocates a new handler array on every ReadingChanged raise, and AccelerometerImplementation.OnChanged calls shakeDetectedSource.HasHandlers for each accelerometer sample; that getter locks and prunes subscriptions. This regresses the previous cheap multicast delegate invocation / null check on a high-frequency sensor path.
💡 Suggestion — HasHandlers documentation is misleading
The XML doc describes HasHandlers as a plain count check and says pruning happens in Subscribe, but the getter itself prunes and Subscribe does not.
Failure-Mode Probing
- Lambda closure subscriber: may be GC’d and stop receiving events unless caller keeps a strong delegate/closure reference.
- Dead page/view-model subscriber: collected successfully; leak fix works.
- Subscriber throws: direct delegate invocation preserves original exception, covered by test.
- No
ShakeDetectedhandlers: processing remains gated, but now pays lock/prune cost per reading.
Verdict: NEEDS_DISCUSSION
Confidence: low
Summary: The leak fix is coherent, but it changes public event lifetime semantics and adds hot-path overhead. CI was not conclusively available locally, so this cannot be LGTM yet.
🛠️ Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | maui-expert-reviewer | Copy-on-write weak target subscriptions with open-instance delegate thunks; no CWT, no closed delegate storage, direct open delegate invocation | ❌ FAILED | 1 file | Runtime nested-generic arity failure, then AOT/trimming analyzer failures (IL2055, IL3050, IL2072) after repair |
| 2 | maui-expert-reviewer | Copy-on-write snapshots while retaining CWT/ephemeron-backed closed delegates for AOT-safe direct invocation | ✅ PASS | 1 file | Focused Accelerometer tests passed 14/14; full Essentials unit suite passed 443/443; optimization tradeoff, not a clear semantic winner |
| PR | PR #36503 | Custom weak event source using weak subscriber targets plus ConditionalWeakTable-held closed delegates, invoked directly; HasHandlers prunes dead entries |
✅ PASSED (Gate) | 3 files | Current PR baseline; simpler and more conservative around stale ShakeDetected gating |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| maui-expert-reviewer | 1 | Yes | Proposed copy-on-write weak targets with open-instance invokers to avoid CWT and per-raise allocation |
| maui-expert-reviewer | 1 failure follow-up | Yes | Diagnosed nested generic arity and suggested the minimal MakeGenericType(typeof(TEventArgs), targetType) repair |
| maui-expert-reviewer | 2 | Yes | After AOT/trimming failure, proposed avoiding dynamic generic construction by retaining CWT ephemeron storage but using copy-on-write snapshots |
| maui-expert-reviewer | 2 comparison | No clear winner | Candidate 2 is a performance-oriented tradeoff; without benchmark evidence, current PR remains more semantically conservative |
Exhausted: Yes — meaningfully different approaches were explored. The pure open-invoker/no-CWT design is blocked by AOT/trimming constraints; the passing copy-on-write/CWT design is not demonstrably better than the PR because it trades hot-path allocation/locking improvements for potentially stale HasHandlers gating. Further options would be minor optimizations of the current PR rather than meaningfully different fixes.
Selected Fix: PR #36503 current fix — it has the supplied passing gate, passes the same conceptual weak-event tests, and preserves stronger HasHandlers pruning semantics. Candidate #2 is a viable performance alternative if future benchmarking proves the current per-reading allocation/lock cost is material.
📝 Recommended PR Title & Description
Assessment: ✏️ Recommend updating — the current description accurately explains the weak-event-source approach but does not mention the winning multicast-delegate correction, and the title can be made more searchable with the component prefix.
Recommended title
[Essentials] Accelerometer: Use weak event sources for ReadingChanged and ShakeDetected
Recommended description
### Issue Details
`Accelerometer.Default` is a singleton instance that persists for the entire lifetime of the application. When a page, view model, or service subscribes to `Accelerometer.ReadingChanged` or `Accelerometer.ShakeDetected` using `+=` but does not explicitly unsubscribe with `-=`, the singleton can retain the subscriber indefinitely. This is easy to overlook because calling `Stop()` may appear to perform cleanup, but it does not remove event subscriptions. The retained subscriber, and any objects it references, are then not eligible for garbage collection.
### Root Cause
`Accelerometer.ReadingChanged` and `Accelerometer.ShakeDetected` were backed by plain multicast delegate events on the app-lifetime `Accelerometer.Default` singleton. A forgotten unsubscribe leaves the singleton's event delegate holding strong references to subscribers for as long as the process runs.
### Description of Change
This replaces the strong event backing fields in `AccelerometerImplementation` with custom `WeakEventSource` / `WeakEventSource<TEventArgs>` helpers for `ReadingChanged` and `ShakeDetected`.
The weak event sources:
- weakly reference instance subscriber targets so forgotten subscribers can be collected normally;
- keep closed delegates in a `ConditionalWeakTable` keyed by the subscriber target, preserving direct delegate invocation without keeping the target alive for the singleton lifetime;
- invoke handlers directly rather than through reflection, so subscriber exceptions propagate with their original type and instance instead of being wrapped in `TargetInvocationException`;
- lazily prune dead entries during subscribe, unsubscribe, raise, and handler checks;
- split multicast delegate subscription and unsubscription into individual invocation-list entries so normal .NET event semantics are preserved and one target in a multicast delegate does not control the lifetime of the others.
This avoids reusing `Microsoft.Maui.WeakEventManager` because Essentials cannot take the required Core dependency without introducing a circular project-reference constraint.
### Tests
Added unit coverage for:
- forgotten `Accelerometer.ReadingChanged` subscribers being collectable;
- still-live subscribers continuing to receive readings after GC;
- direct propagation of subscriber exceptions;
- multicast delegate weak-event behavior so earlier targets are not leaked and live earlier handlers are not dropped when later targets are collected.
Validated the behavior on:
- [x] Android
- [x] Windows
- [x] iOS
- [x] Mac
### Issues Fixed
Fixes https://github.com/dotnet/maui/issues/36216
### Output ScreenShot
| Platform | Before | After |
|--|--|--|
| Android | <video src="https://github.com/user-attachments/assets/708f2d8e-8ee4-495c-a2a3-cb3771532121" > | <video src="https://github.com/user-attachments/assets/3d787398-b7fa-43d4-a362-9f1d1eef6380"> |
| Mac | <video src="https://github.com/user-attachments/assets/4236c5ed-dbdc-4002-95b6-e08dc6c95ca2"> | <video src="https://github.com/user-attachments/assets/13475a2c-35a6-4065-8097-d62d68e7be76"> |
| iOS | <video src="https://github.com/user-attachments/assets/f54329ba-5c30-47d3-94d0-38093ba92cf4"> | <video src="https://github.com/user-attachments/assets/6f75dc4e-8ca5-47a2-b357-16fa906b9add"> |
| Windows | <video src="https://github.com/user-attachments/assets/e5e9455a-58c7-4ca8-9e2b-a15b4f6ba271"> | <video src="https://github.com/user-attachments/assets/2cfc5011-f79a-45d4-aa18-94abd8f6b576"> |
🏁 Report — Final Recommendation
Comparative report — PR #36503
Candidate ranking
| Rank | Candidate | Regression tests | Assessment |
|---|---|---|---|
| 1 | pr-plus-reviewer |
✅ Passed | Best candidate. It preserves the PR's AOT-safe ConditionalWeakTable weak-lifetime design and direct delegate invocation, while fixing the expert-reviewed multicast delegate bug. Sandbox focused Accelerometer tests passed 16/16 and full Essentials unit tests passed 445/445. |
| 2 | try-fix-2 |
✅ Passed | Viable performance-oriented alternative. It also splits multicast invocation lists and avoids per-raise handler-array allocation via copy-on-write snapshots, but trades away the PR's conservative HasHandlers pruning: a dead ShakeDetected subscriber can keep shake processing enabled until a later raise/unsubscribe prunes it. Without benchmark evidence that the PR hot-path cost is material, this is not a better default than the smaller reviewer patch. |
| 3 | pr |
✅ Passed supplied gate | The raw PR fixes the core forgotten-subscriber leak and preserves conservative HasHandlers pruning, but the expert review found a concrete multicast delegate correctness/lifetime bug. It should rank below passing candidates that address that issue. |
| 4 | try-fix-1 |
❌ Failed | Not viable. The open-instance invoker/no-ConditionalWeakTable approach first failed at runtime due to nested generic arity and then failed AOT/trimming analyzers (IL2055, IL3050, IL2072) after repair. Per rule, failed-regression candidates rank below passing candidates. |
Why pr-plus-reviewer wins
pr-plus-reviewer is the most complete and least risky fix. It keeps the submitted PR's core design: weak target references, ephemeron-backed closed delegates via ConditionalWeakTable, direct invocation without reflection, lazy pruning, and HasHandlers pruning before shake processing. It then adds the missing normal .NET event semantic: if a caller subscribes a multicast delegate, each invocation-list entry is stored and removed independently.
This resolves the expert reviewer's actionable defect without adopting try-fix-2's broader copy-on-write rewrite or its weaker HasHandlers semantics. It also adds regression tests for the exact multicast failure modes and passes both focused and full Essentials unit suites in the sandbox.
Candidate-by-candidate notes
pr
The submitted PR is directionally correct and passed the supplied gate. Its weakness is in treating a potentially multicast delegate as a single subscription. Because the ConditionalWeakTable key is the last target while the stored closed multicast delegate can strongly reference earlier targets, the raw PR can still leak earlier targets or drop live earlier handlers when the last target dies.
pr-plus-reviewer
The reviewer patch is surgical: split handler.GetInvocationList() in both generic and non-generic subscribe/unsubscribe paths and keep the existing single-cast storage logic unchanged. This fixes the raw PR's multicast bug while preserving AOT/trimming safety and existing pruning behavior.
try-fix-1
The pure open-invoker/no-CWT design is not viable for Essentials because it depends on runtime generic type construction and activation patterns that fail AOT/trimming analysis.
try-fix-2
The copy-on-write/CWT design is viable and passed tests, but it is a larger rewrite whose main advantage is performance-oriented. Its HasHandlers check is O(1), but that means stale dead ShakeDetected subscriptions can keep the shake-processing gate open until a later prune path runs. The smaller pr-plus-reviewer fix is preferable absent benchmark evidence.
Winner
Winner: pr-plus-reviewer
Rationale: It is a PR-based fix that incorporates the expert reviewer's concrete correctness feedback, preserves the safest parts of the original PR, avoids the failed AOT/trimming approach, and passes the relevant sandbox test suites.
🧭 Next Steps — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
MauiBot
removed
the
s/agent-review-in-progress
…using weak event source (#36503) <!-- Please let the below note in for people that find this PR --> > [!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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details The Accelerometer.Default is a singleton instance that persists for the entire lifetime of the application. When a page, view model, or service subscribes to Accelerometer.ReadingChanged or ShakeDetected using += but does not explicitly unsubscribe with -= — a common oversight, since calling Stop() may appear to perform cleanup but does not remove the event subscription — the singleton retains a strong reference to the subscriber indefinitely. Consequently, the subscriber, along with any objects it references, is never eligible for garbage collection. This results in a progressively growing memory leak each time the pattern recurs. ### Root Cause The issue occurs because Accelerometer.ReadingChanged and ShakeDetected being backed by plain multicast delegate events on Accelerometer.Default , which is an app-lifetime static singleton. When a subscriber registers with += but never explicitly unsubscribes with -= (a common mistake, since calling Stop() looks like cleanup but does not clear the delegate), the singleton's multicast delegate holds a strong reference to the subscriber for as long as the process runs, preventing it from ever being garbage collected — even after the subscriber's owning page, view model, or service should have gone out of scope. ### Description of Change The fix involves replacing the plain multicast delegate events with a custom WeakEventSource / WeakEventSource<TEventArgs> type that weakly references each subscriber's target object while keeping its closed delegate in a ConditionalWeakTable keyed by that target, invoking handlers via direct delegate calls rather than reflection. This avoids the MethodInfo.Invoke overhead and exception-wrapping behavior of the pattern used by Microsoft.Maui.WeakEventManager elsewhere in the codebase (not reused directly here due to a circular project-reference constraint between Essentials and Core), so subscriber exceptions propagate with their original type and instance intact. Dead entries are pruned lazily during Subscribe , Unsubscribe , Raise , and HasHandlers . This allows a forgotten subscriber to be collected normally while still-alive subscribers continue to receive events correctly, and ensures a collected subscriber does not keep shake processing enabled indefinitely — verified by unit tests covering the leak scenario, the live-subscriber-after-GC scenario, and direct exception propagation. Validated the behavior in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Issues Fixed Fixes #36216 ### Output ScreenShot |Platform|Before|After| |--|--|--| |Android| <video src="https://github.com/user-attachments/assets/708f2d8e-8ee4-495c-a2a3-cb3771532121" >| <video src="https://github.com/user-attachments/assets/3d787398-b7fa-43d4-a362-9f1d1eef6380">| |Mac|<video src="https://github.com/user-attachments/assets/4236c5ed-dbdc-4002-95b6-e08dc6c95ca2"> |<video src="https://github.com/user-attachments/assets/13475a2c-35a6-4065-8097-d62d68e7be76">| |iOS|<video src="https://github.com/user-attachments/assets/f54329ba-5c30-47d3-94d0-38093ba92cf4">|<video src="https://github.com/user-attachments/assets/6f75dc4e-8ca5-47a2-b357-16fa906b9add">| |Windows|<video src="https://github.com/user-attachments/assets/e5e9455a-58c7-4ca8-9e2b-a15b4f6ba271">|<video src="https://github.com/user-attachments/assets/2cfc5011-f79a-45d4-aa18-94abd8f6b576"> |
…using weak event source (#36503) <!-- Please let the below note in for people that find this PR --> > [!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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details The Accelerometer.Default is a singleton instance that persists for the entire lifetime of the application. When a page, view model, or service subscribes to Accelerometer.ReadingChanged or ShakeDetected using += but does not explicitly unsubscribe with -= — a common oversight, since calling Stop() may appear to perform cleanup but does not remove the event subscription — the singleton retains a strong reference to the subscriber indefinitely. Consequently, the subscriber, along with any objects it references, is never eligible for garbage collection. This results in a progressively growing memory leak each time the pattern recurs. ### Root Cause The issue occurs because Accelerometer.ReadingChanged and ShakeDetected being backed by plain multicast delegate events on Accelerometer.Default , which is an app-lifetime static singleton. When a subscriber registers with += but never explicitly unsubscribes with -= (a common mistake, since calling Stop() looks like cleanup but does not clear the delegate), the singleton's multicast delegate holds a strong reference to the subscriber for as long as the process runs, preventing it from ever being garbage collected — even after the subscriber's owning page, view model, or service should have gone out of scope. ### Description of Change The fix involves replacing the plain multicast delegate events with a custom WeakEventSource / WeakEventSource<TEventArgs> type that weakly references each subscriber's target object while keeping its closed delegate in a ConditionalWeakTable keyed by that target, invoking handlers via direct delegate calls rather than reflection. This avoids the MethodInfo.Invoke overhead and exception-wrapping behavior of the pattern used by Microsoft.Maui.WeakEventManager elsewhere in the codebase (not reused directly here due to a circular project-reference constraint between Essentials and Core), so subscriber exceptions propagate with their original type and instance intact. Dead entries are pruned lazily during Subscribe , Unsubscribe , Raise , and HasHandlers . This allows a forgotten subscriber to be collected normally while still-alive subscribers continue to receive events correctly, and ensures a collected subscriber does not keep shake processing enabled indefinitely — verified by unit tests covering the leak scenario, the live-subscriber-after-GC scenario, and direct exception propagation. Validated the behavior in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Issues Fixed Fixes #36216 ### Output ScreenShot |Platform|Before|After| |--|--|--| |Android| <video src="https://github.com/user-attachments/assets/708f2d8e-8ee4-495c-a2a3-cb3771532121" >| <video src="https://github.com/user-attachments/assets/3d787398-b7fa-43d4-a362-9f1d1eef6380">| |Mac|<video src="https://github.com/user-attachments/assets/4236c5ed-dbdc-4002-95b6-e08dc6c95ca2"> |<video src="https://github.com/user-attachments/assets/13475a2c-35a6-4065-8097-d62d68e7be76">| |iOS|<video src="https://github.com/user-attachments/assets/f54329ba-5c30-47d3-94d0-38093ba92cf4">|<video src="https://github.com/user-attachments/assets/6f75dc4e-8ca5-47a2-b357-16fa906b9add">| |Windows|<video src="https://github.com/user-attachments/assets/e5e9455a-58c7-4ca8-9e2b-a15b4f6ba271">|<video src="https://github.com/user-attachments/assets/2cfc5011-f79a-45d4-aa18-94abd8f6b576"> |
…using weak event source (#36503) <!-- Please let the below note in for people that find this PR --> > [!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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details The Accelerometer.Default is a singleton instance that persists for the entire lifetime of the application. When a page, view model, or service subscribes to Accelerometer.ReadingChanged or ShakeDetected using += but does not explicitly unsubscribe with -= — a common oversight, since calling Stop() may appear to perform cleanup but does not remove the event subscription — the singleton retains a strong reference to the subscriber indefinitely. Consequently, the subscriber, along with any objects it references, is never eligible for garbage collection. This results in a progressively growing memory leak each time the pattern recurs. ### Root Cause The issue occurs because Accelerometer.ReadingChanged and ShakeDetected being backed by plain multicast delegate events on Accelerometer.Default , which is an app-lifetime static singleton. When a subscriber registers with += but never explicitly unsubscribes with -= (a common mistake, since calling Stop() looks like cleanup but does not clear the delegate), the singleton's multicast delegate holds a strong reference to the subscriber for as long as the process runs, preventing it from ever being garbage collected — even after the subscriber's owning page, view model, or service should have gone out of scope. ### Description of Change The fix involves replacing the plain multicast delegate events with a custom WeakEventSource / WeakEventSource<TEventArgs> type that weakly references each subscriber's target object while keeping its closed delegate in a ConditionalWeakTable keyed by that target, invoking handlers via direct delegate calls rather than reflection. This avoids the MethodInfo.Invoke overhead and exception-wrapping behavior of the pattern used by Microsoft.Maui.WeakEventManager elsewhere in the codebase (not reused directly here due to a circular project-reference constraint between Essentials and Core), so subscriber exceptions propagate with their original type and instance intact. Dead entries are pruned lazily during Subscribe , Unsubscribe , Raise , and HasHandlers . This allows a forgotten subscriber to be collected normally while still-alive subscribers continue to receive events correctly, and ensures a collected subscriber does not keep shake processing enabled indefinitely — verified by unit tests covering the leak scenario, the live-subscriber-after-GC scenario, and direct exception propagation. Validated the behavior in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Issues Fixed Fixes #36216 ### Output ScreenShot |Platform|Before|After| |--|--|--| |Android| <video src="https://github.com/user-attachments/assets/708f2d8e-8ee4-495c-a2a3-cb3771532121" >| <video src="https://github.com/user-attachments/assets/3d787398-b7fa-43d4-a362-9f1d1eef6380">| |Mac|<video src="https://github.com/user-attachments/assets/4236c5ed-dbdc-4002-95b6-e08dc6c95ca2"> |<video src="https://github.com/user-attachments/assets/13475a2c-35a6-4065-8097-d62d68e7be76">| |iOS|<video src="https://github.com/user-attachments/assets/f54329ba-5c30-47d3-94d0-38093ba92cf4">|<video src="https://github.com/user-attachments/assets/6f75dc4e-8ca5-47a2-b357-16fa906b9add">| |Windows|<video src="https://github.com/user-attachments/assets/e5e9455a-58c7-4ca8-9e2b-a15b4f6ba271">|<video src="https://github.com/user-attachments/assets/2cfc5011-f79a-45d4-aa18-94abd8f6b576"> |
…using weak event source (#36503) <!-- Please let the below note in for people that find this PR --> > [!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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details The Accelerometer.Default is a singleton instance that persists for the entire lifetime of the application. When a page, view model, or service subscribes to Accelerometer.ReadingChanged or ShakeDetected using += but does not explicitly unsubscribe with -= — a common oversight, since calling Stop() may appear to perform cleanup but does not remove the event subscription — the singleton retains a strong reference to the subscriber indefinitely. Consequently, the subscriber, along with any objects it references, is never eligible for garbage collection. This results in a progressively growing memory leak each time the pattern recurs. ### Root Cause The issue occurs because Accelerometer.ReadingChanged and ShakeDetected being backed by plain multicast delegate events on Accelerometer.Default , which is an app-lifetime static singleton. When a subscriber registers with += but never explicitly unsubscribes with -= (a common mistake, since calling Stop() looks like cleanup but does not clear the delegate), the singleton's multicast delegate holds a strong reference to the subscriber for as long as the process runs, preventing it from ever being garbage collected — even after the subscriber's owning page, view model, or service should have gone out of scope. ### Description of Change The fix involves replacing the plain multicast delegate events with a custom WeakEventSource / WeakEventSource<TEventArgs> type that weakly references each subscriber's target object while keeping its closed delegate in a ConditionalWeakTable keyed by that target, invoking handlers via direct delegate calls rather than reflection. This avoids the MethodInfo.Invoke overhead and exception-wrapping behavior of the pattern used by Microsoft.Maui.WeakEventManager elsewhere in the codebase (not reused directly here due to a circular project-reference constraint between Essentials and Core), so subscriber exceptions propagate with their original type and instance intact. Dead entries are pruned lazily during Subscribe , Unsubscribe , Raise , and HasHandlers . This allows a forgotten subscriber to be collected normally while still-alive subscribers continue to receive events correctly, and ensures a collected subscriber does not keep shake processing enabled indefinitely — verified by unit tests covering the leak scenario, the live-subscriber-after-GC scenario, and direct exception propagation. Validated the behavior in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Issues Fixed Fixes #36216 ### Output ScreenShot |Platform|Before|After| |--|--|--| |Android| <video src="https://github.com/user-attachments/assets/708f2d8e-8ee4-495c-a2a3-cb3771532121" >| <video src="https://github.com/user-attachments/assets/3d787398-b7fa-43d4-a362-9f1d1eef6380">| |Mac|<video src="https://github.com/user-attachments/assets/4236c5ed-dbdc-4002-95b6-e08dc6c95ca2"> |<video src="https://github.com/user-attachments/assets/13475a2c-35a6-4065-8097-d62d68e7be76">| |iOS|<video src="https://github.com/user-attachments/assets/f54329ba-5c30-47d3-94d0-38093ba92cf4">|<video src="https://github.com/user-attachments/assets/6f75dc4e-8ca5-47a2-b357-16fa906b9add">| |Windows|<video src="https://github.com/user-attachments/assets/e5e9455a-58c7-4ca8-9e2b-a15b4f6ba271">|<video src="https://github.com/user-attachments/assets/2cfc5011-f79a-45d4-aa18-94abd8f6b576"> |
…using weak event source (#36503) <!-- Please let the below note in for people that find this PR --> > [!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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details The Accelerometer.Default is a singleton instance that persists for the entire lifetime of the application. When a page, view model, or service subscribes to Accelerometer.ReadingChanged or ShakeDetected using += but does not explicitly unsubscribe with -= — a common oversight, since calling Stop() may appear to perform cleanup but does not remove the event subscription — the singleton retains a strong reference to the subscriber indefinitely. Consequently, the subscriber, along with any objects it references, is never eligible for garbage collection. This results in a progressively growing memory leak each time the pattern recurs. ### Root Cause The issue occurs because Accelerometer.ReadingChanged and ShakeDetected being backed by plain multicast delegate events on Accelerometer.Default , which is an app-lifetime static singleton. When a subscriber registers with += but never explicitly unsubscribes with -= (a common mistake, since calling Stop() looks like cleanup but does not clear the delegate), the singleton's multicast delegate holds a strong reference to the subscriber for as long as the process runs, preventing it from ever being garbage collected — even after the subscriber's owning page, view model, or service should have gone out of scope. ### Description of Change The fix involves replacing the plain multicast delegate events with a custom WeakEventSource / WeakEventSource<TEventArgs> type that weakly references each subscriber's target object while keeping its closed delegate in a ConditionalWeakTable keyed by that target, invoking handlers via direct delegate calls rather than reflection. This avoids the MethodInfo.Invoke overhead and exception-wrapping behavior of the pattern used by Microsoft.Maui.WeakEventManager elsewhere in the codebase (not reused directly here due to a circular project-reference constraint between Essentials and Core), so subscriber exceptions propagate with their original type and instance intact. Dead entries are pruned lazily during Subscribe , Unsubscribe , Raise , and HasHandlers . This allows a forgotten subscriber to be collected normally while still-alive subscribers continue to receive events correctly, and ensures a collected subscriber does not keep shake processing enabled indefinitely — verified by unit tests covering the leak scenario, the live-subscriber-after-GC scenario, and direct exception propagation. Validated the behavior in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Issues Fixed Fixes #36216 ### Output ScreenShot |Platform|Before|After| |--|--|--| |Android| <video src="https://github.com/user-attachments/assets/708f2d8e-8ee4-495c-a2a3-cb3771532121" >| <video src="https://github.com/user-attachments/assets/3d787398-b7fa-43d4-a362-9f1d1eef6380">| |Mac|<video src="https://github.com/user-attachments/assets/4236c5ed-dbdc-4002-95b6-e08dc6c95ca2"> |<video src="https://github.com/user-attachments/assets/13475a2c-35a6-4065-8097-d62d68e7be76">| |iOS|<video src="https://github.com/user-attachments/assets/f54329ba-5c30-47d3-94d0-38093ba92cf4">|<video src="https://github.com/user-attachments/assets/6f75dc4e-8ca5-47a2-b357-16fa906b9add">| |Windows|<video src="https://github.com/user-attachments/assets/e5e9455a-58c7-4ca8-9e2b-a15b4f6ba271">|<video src="https://github.com/user-attachments/assets/2cfc5011-f79a-45d4-aa18-94abd8f6b576"> |
…using weak event source (#36503) <!-- Please let the below note in for people that find this PR --> > [!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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details The Accelerometer.Default is a singleton instance that persists for the entire lifetime of the application. When a page, view model, or service subscribes to Accelerometer.ReadingChanged or ShakeDetected using += but does not explicitly unsubscribe with -= — a common oversight, since calling Stop() may appear to perform cleanup but does not remove the event subscription — the singleton retains a strong reference to the subscriber indefinitely. Consequently, the subscriber, along with any objects it references, is never eligible for garbage collection. This results in a progressively growing memory leak each time the pattern recurs. ### Root Cause The issue occurs because Accelerometer.ReadingChanged and ShakeDetected being backed by plain multicast delegate events on Accelerometer.Default , which is an app-lifetime static singleton. When a subscriber registers with += but never explicitly unsubscribes with -= (a common mistake, since calling Stop() looks like cleanup but does not clear the delegate), the singleton's multicast delegate holds a strong reference to the subscriber for as long as the process runs, preventing it from ever being garbage collected — even after the subscriber's owning page, view model, or service should have gone out of scope. ### Description of Change The fix involves replacing the plain multicast delegate events with a custom WeakEventSource / WeakEventSource<TEventArgs> type that weakly references each subscriber's target object while keeping its closed delegate in a ConditionalWeakTable keyed by that target, invoking handlers via direct delegate calls rather than reflection. This avoids the MethodInfo.Invoke overhead and exception-wrapping behavior of the pattern used by Microsoft.Maui.WeakEventManager elsewhere in the codebase (not reused directly here due to a circular project-reference constraint between Essentials and Core), so subscriber exceptions propagate with their original type and instance intact. Dead entries are pruned lazily during Subscribe , Unsubscribe , Raise , and HasHandlers . This allows a forgotten subscriber to be collected normally while still-alive subscribers continue to receive events correctly, and ensures a collected subscriber does not keep shake processing enabled indefinitely — verified by unit tests covering the leak scenario, the live-subscriber-after-GC scenario, and direct exception propagation. Validated the behavior in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Issues Fixed Fixes #36216 ### Output ScreenShot |Platform|Before|After| |--|--|--| |Android| <video src="https://github.com/user-attachments/assets/708f2d8e-8ee4-495c-a2a3-cb3771532121" >| <video src="https://github.com/user-attachments/assets/3d787398-b7fa-43d4-a362-9f1d1eef6380">| |Mac|<video src="https://github.com/user-attachments/assets/4236c5ed-dbdc-4002-95b6-e08dc6c95ca2"> |<video src="https://github.com/user-attachments/assets/13475a2c-35a6-4065-8097-d62d68e7be76">| |iOS|<video src="https://github.com/user-attachments/assets/f54329ba-5c30-47d3-94d0-38093ba92cf4">|<video src="https://github.com/user-attachments/assets/6f75dc4e-8ca5-47a2-b357-16fa906b9add">| |Windows|<video src="https://github.com/user-attachments/assets/e5e9455a-58c7-4ca8-9e2b-a15b4f6ba271">|<video src="https://github.com/user-attachments/assets/2cfc5011-f79a-45d4-aa18-94abd8f6b576"> |
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
The Accelerometer.Default is a singleton instance that persists for the entire lifetime of the application. When a page, view model, or service subscribes to Accelerometer.ReadingChanged or ShakeDetected using += but does not explicitly unsubscribe with -= — a common oversight, since calling Stop() may appear to perform cleanup but does not remove the event subscription — the singleton retains a strong reference to the subscriber indefinitely. Consequently, the subscriber, along with any objects it references, is never eligible for garbage collection. This results in a progressively growing memory leak each time the pattern recurs.
Root Cause
The issue occurs because Accelerometer.ReadingChanged and ShakeDetected being backed by plain multicast delegate events on Accelerometer.Default , which is an app-lifetime static singleton. When a subscriber registers with += but never explicitly unsubscribes with -= (a common mistake, since calling Stop() looks like cleanup but does not clear the delegate), the singleton's multicast delegate holds a strong reference to the subscriber for as long as the process runs, preventing it from ever being garbage collected — even after the subscriber's owning page, view model, or service should have gone out of scope.
Description of Change
The fix involves replacing the plain multicast delegate events with a custom WeakEventSource / WeakEventSource type that weakly references each subscriber's target object while keeping its closed delegate in a ConditionalWeakTable keyed by that target, invoking handlers via direct delegate calls rather than reflection. This avoids the MethodInfo.Invoke overhead and exception-wrapping behavior of the pattern used by Microsoft.Maui.WeakEventManager elsewhere in the codebase (not reused directly here due to a circular project-reference constraint between Essentials and Core), so subscriber exceptions propagate with their original type and instance intact.
Dead entries are pruned lazily during Subscribe , Unsubscribe , Raise , and HasHandlers . This allows a forgotten subscriber to be collected normally while still-alive subscribers continue to receive events correctly, and ensures a collected subscriber does not keep shake processing enabled indefinitely — verified by unit tests covering the leak scenario, the live-subscriber-after-GC scenario, and direct exception propagation.
Validated the behavior in the following platforms
Issues Fixed
Fixes #36216
Output ScreenShot
36216-BeforeFix.Android.mov
36216-AfterFix.Android.mov
36216-BeforeFix.mac.mov
36216-AfterFix.mac.mov
36216-BeforeFix.iOS.mov
36216-AfterFix.iOS.mov
36216-BeforeFix.Windows.mp4
36216-AfterFix.Windows.mp4