-
Notifications
You must be signed in to change notification settings - Fork 2k
[Android] Fix ActivityStateManager leaking lifecycle callbacks on Activity recreation #36161
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
104 changes: 104 additions & 0 deletions
104
src/Essentials/test/DeviceTests/Tests/ActivityStateManager_Tests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| // Device tests for GitHub issue #36035: | ||
| // ActivityStateManager listener leak — Init(Application) was called on every | ||
| // Activity recreation without a guard, registering a new listener each time. | ||
| // Android's RegisterActivityLifecycleCallbacks is additive, so all old listeners | ||
| // accumulated and every ActivityStateChanged event fired N+1 times after N recreations. | ||
|
|
||
| #if __ANDROID__ | ||
| using System.Linq; | ||
| using System.Threading; | ||
| using Microsoft.Maui.ApplicationModel; | ||
| using Xunit; | ||
|
|
||
| namespace Microsoft.Maui.Essentials.DeviceTests | ||
| { | ||
| using MauiPlatform = Microsoft.Maui.ApplicationModel.Platform; | ||
|
|
||
| [Category("ActivityStateManager")] | ||
| public class ActivityStateManager_Tests | ||
| { | ||
| /// <summary> | ||
| /// Verifies the fix for issue #36035: calling Init(Application) multiple times | ||
| /// (which happens on every Activity recreation) must keep the SAME listener | ||
| /// and NOT register additional ones. | ||
| /// Before the fix: each call created a new listener → N calls = N listeners. | ||
| /// After the fix: the guard `if (lifecycleListener is not null) return;` ensures | ||
| /// only the first call registers a listener. | ||
| /// </summary> | ||
| [Fact] | ||
| public void Init_CalledMultipleTimes_SameListenerIsKept() | ||
| { | ||
| var app = (global::Android.App.Application)global::Android.App.Application.Context; | ||
| var manager = new ActivityStateManagerImplementation(); | ||
|
|
||
| manager.Init(app); | ||
| var listenerAfterFirst = GetListener(manager); | ||
|
|
||
| manager.Init(app); | ||
| var listenerAfterSecond = GetListener(manager); | ||
|
|
||
| manager.Init(app); | ||
| var listenerAfterThird = GetListener(manager); | ||
|
|
||
| Assert.NotNull(listenerAfterFirst); | ||
| Assert.Same(listenerAfterFirst, listenerAfterSecond); | ||
| Assert.Same(listenerAfterFirst, listenerAfterThird); | ||
| } | ||
|
Comment on lines
+29
to
+46
|
||
|
|
||
| /// <summary> | ||
| /// Verifies that ActivityStateChanged fires exactly once per lifecycle event | ||
| /// even after Init(Application) is called multiple times. | ||
| /// Before the fix: N calls = N listeners → event fired N times per real event. | ||
| /// After the fix: always exactly 1 listener → event fires exactly once. | ||
| /// | ||
| /// IMPORTANT: we capture each listener BEFORE the next Init call because | ||
| /// the bug overwrites the internal field each time. We then fire on ALL captured | ||
| /// listeners (simulating Android dispatching to every registered callback). | ||
| /// With the fix all three are the same object → fires once. | ||
| /// With the bug all three are different objects → fires three times. | ||
| /// </summary> | ||
| [Fact] | ||
| public void Init_CalledMultipleTimes_ActivityStateChangedFiresOnce() | ||
| { | ||
| var app = (global::Android.App.Application)global::Android.App.Application.Context; | ||
| var activity = MauiPlatform.CurrentActivity; | ||
|
|
||
| var manager = new ActivityStateManagerImplementation(); | ||
|
|
||
| int invocations = 0; | ||
| manager.ActivityStateChanged += (_, _) => Interlocked.Increment(ref invocations); | ||
|
|
||
| // Capture the listener after EACH Init call, before the next one overwrites it. | ||
| manager.Init(app); | ||
| var l1 = GetListener(manager) as global::Android.App.Application.IActivityLifecycleCallbacks; | ||
|
|
||
| manager.Init(app); | ||
| var l2 = GetListener(manager) as global::Android.App.Application.IActivityLifecycleCallbacks; | ||
|
|
||
| manager.Init(app); | ||
| var l3 = GetListener(manager) as global::Android.App.Application.IActivityLifecycleCallbacks; | ||
|
|
||
| // Simulate Android dispatching a Resumed event to every distinct registered listener. | ||
| // With fix: l1 == l2 == l3 (same object) → 1 unique listener → 1 invocation ✅ | ||
| // With bug: l1 != l2 != l3 (different) → 3 unique listeners → 3 invocations ❌ | ||
| foreach (var l in new[] { l1, l2, l3 }.Distinct()) | ||
| l?.OnActivityResumed(activity); | ||
|
|
||
| Assert.Equal(1, invocations); | ||
| } | ||
|
Comment on lines
+64
to
+88
|
||
|
|
||
| // Reads the private 'lifecycleListener' field from ActivityStateManagerImplementation | ||
| // via reflection. InternalsVisibleTo in AssemblyInfo.shared.cs grants access to | ||
| // internal types; reflection is needed only for the private field itself. | ||
| static ActivityLifecycleContextListener GetListener(ActivityStateManagerImplementation manager) | ||
| { | ||
| var field = typeof(ActivityStateManagerImplementation) | ||
| .GetField("lifecycleListener", | ||
| System.Reflection.BindingFlags.NonPublic | | ||
| System.Reflection.BindingFlags.Instance); | ||
|
|
||
| return field?.GetValue(manager) as ActivityLifecycleContextListener; | ||
| } | ||
| } | ||
| } | ||
| #endif | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[moderate] Regression Prevention / Memory Leak — These tests register a real
Application.IActivityLifecycleCallbacksinstance on the process-wide AndroidApplication, but never unregister it.RegisterActivityLifecycleCallbacksis additive and theApplicationlives for the whole device-test process, so each test leaves behind a listener that still references itsActivityStateManagerImplementationand receives future activity lifecycle events. This can make later tests observe extra callbacks and prevents the test manager from being collected.Please unregister the listener in
finally(and do the same for the listener created around line 72), e.g. retrieveGetListener(manager)asApplication.IActivityLifecycleCallbacksand callapp.UnregisterActivityLifecycleCallbacks(listener)after assertions.