Merge pull request #557 from Countly/fix556/memory_leak

arifBurakDemiray · web-flow · commit 392a298ae87b · 2026-05-11T10:32:49.000+03:00
Fix556/memory leak
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,8 @@
 * Added gradle configuration cache support to upload symbols plugin.
 * Improved user properties auto-save conditions to flush event queue with every user property call.
 
+* Mitigated a memory leak where the content overlay retained the activity it was first opened in across subsequent activity transitions.
+
 ## 26.1.2
 * Added `CountlyInitProvider` ContentProvider to register activity lifecycle callbacks before `Application.onCreate()`. This ensures the SDK captures the current activity in single-activity frameworks (Flutter, React Native) and apps with deferred initialization.
 * Added `CountlyConfig.setInitialActivity(Activity)` as an explicit way for wrapper SDKs to provide the host activity during initialization.
diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/ContentOverlayViewTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/ContentOverlayViewTests.java
@@ -790,4 +790,48 @@ public void contentUrlAction_noQueryParams_returnsFalse() {
             Assert.assertFalse(overlay.contentUrlAction(url, overlay.webView));
         });
     }
+
+    // ===================== Memory leak prevention (issue #556) =====================
+
+    /**
+     * Structural invariant: the overlay's View.mContext must be the Application, not the
+     * constructing activity. This is what allows the overlay to outlive activity transitions
+     * without leaking the activity it was first opened in.
+     *
+     * Regression guard: if anyone changes the constructor's super(...) call back to the
+     * activity, this test will fail and surface the leak before users do.
+     */
+    @Test
+    public void constructor_usesApplicationContext_notActivity() {
+        withActivity(activity -> {
+            overlay = createOverlay(activity);
+            Assert.assertNotSame(
+                "ContentOverlayView.mContext must not be the constructing Activity — "
+                    + "View.mContext can never be swapped, so binding it to an Activity leaks "
+                    + "that Activity for the lifetime of the overlay.",
+                activity, overlay.getContext());
+            Assert.assertSame(
+                "ContentOverlayView.mContext must be the Application context.",
+                activity.getApplicationContext(), overlay.getContext());
+        });
+    }
+
+    /**
+     * Same invariant for the embedded WebView. Even with the wrapper View using App context,
+     * a WebView constructed with Activity context would still pin the constructing activity
+     * via its own mContext.
+     */
+    @Test
+    public void webView_usesApplicationContext_notActivity() {
+        withActivity(activity -> {
+            overlay = createOverlay(activity);
+            Assert.assertNotNull("WebView should be created during construction", overlay.webView);
+            Assert.assertNotSame(
+                "ContentOverlayView's WebView.mContext must not be the constructing Activity.",
+                activity, overlay.webView.getContext());
+            Assert.assertSame(
+                "ContentOverlayView's WebView.mContext must be the Application context.",
+                activity.getApplicationContext(), overlay.webView.getContext());
+        });
+    }
 }
diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/ModuleContentTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleContentTests.java
@@ -1,5 +1,6 @@
 package ly.count.android.sdk;
 
+import android.app.Activity;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
 import java.util.ArrayList;
 import java.util.List;
@@ -11,6 +12,8 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import static org.mockito.Mockito.mock;
+
 @RunWith(AndroidJUnit4.class)
 public class ModuleContentTests {
 
@@ -68,6 +71,12 @@ private void setIsCurrentlyInContentZone(ModuleContent module, boolean value) th
         field.set(module, value);
     }
 
+    private Activity getCurrentActivity(ModuleContent module) throws Exception {
+        java.lang.reflect.Field field = ModuleContent.class.getDeclaredField("currentActivity");
+        field.setAccessible(true);
+        return (Activity) field.get(module);
+    }
+
     // ======== previewContent public API tests ========
 
     /**
@@ -158,4 +167,92 @@ public void validateResponse() throws JSONException {
         valid.put("html", "<html></html>");
         Assert.assertTrue(mc.validateResponse(valid));
     }
+
+    // ======== Activity reference / leak prevention tests (issue #556) ========
+
+    /**
+     * onActivityDestroyed must null out currentActivity when the destroyed activity
+     * is the one currently tracked. This is the core leak fix.
+     */
+    @Test
+    public void onActivityDestroyed_clearsCurrentActivity_whenIdentityMatches() throws Exception {
+        Countly countly = initWithConsent(true);
+        ModuleContent mc = countly.moduleContent;
+
+        Activity act = mock(Activity.class);
+        mc.onActivityStarted(act, 1);
+        Assert.assertSame(act, getCurrentActivity(mc));
+
+        mc.onActivityDestroyed(act);
+        Assert.assertNull(getCurrentActivity(mc));
+    }
+
+    /**
+     * Destroying an activity other than the currently tracked one must NOT clear the field.
+     * This protects against losing the active activity reference when an old, already-replaced
+     * activity is finally destroyed.
+     */
+    @Test
+    public void onActivityDestroyed_doesNotClear_whenDifferentActivity() throws Exception {
+        Countly countly = initWithConsent(true);
+        ModuleContent mc = countly.moduleContent;
+
+        Activity tracked = mock(Activity.class);
+        Activity unrelated = mock(Activity.class);
+        mc.onActivityStarted(tracked, 1);
+
+        mc.onActivityDestroyed(unrelated);
+        Assert.assertSame(tracked, getCurrentActivity(mc));
+    }
+
+    /**
+     * Rotation race regression: when onActivityStarted for the new activity fires before
+     * onActivityDestroyed for the old one, destroying the old activity must not wipe out
+     * the new tracked activity.
+     */
+    @Test
+    public void onActivityDestroyed_doesNotClearNewerActivity_afterRotationRace() throws Exception {
+        Countly countly = initWithConsent(true);
+        ModuleContent mc = countly.moduleContent;
+
+        Activity oldAct = mock(Activity.class);
+        Activity newAct = mock(Activity.class);
+
+        mc.onActivityStarted(oldAct, 1);
+        mc.onActivityStarted(newAct, 2);
+        Assert.assertSame(newAct, getCurrentActivity(mc));
+
+        // Old activity is finally destroyed after the new one has already taken over.
+        mc.onActivityDestroyed(oldAct);
+        Assert.assertSame(newAct, getCurrentActivity(mc));
+    }
+
+    /**
+     * onActivityDestroyed must not throw when no activity has been tracked yet.
+     */
+    @Test
+    public void onActivityDestroyed_isSafe_whenNoActivityTracked() throws Exception {
+        Countly countly = initWithConsent(true);
+        ModuleContent mc = countly.moduleContent;
+
+        Activity stray = mock(Activity.class);
+        mc.onActivityDestroyed(stray);
+        Assert.assertNull(getCurrentActivity(mc));
+    }
+
+    /**
+     * The seeded activity path (onInitialActivitySeeded) must also be cleared on destroy.
+     */
+    @Test
+    public void onActivityDestroyed_clearsSeededActivity() throws Exception {
+        Countly countly = initWithConsent(true);
+        ModuleContent mc = countly.moduleContent;
+
+        Activity seeded = mock(Activity.class);
+        mc.onInitialActivitySeeded(seeded);
+        Assert.assertSame(seeded, getCurrentActivity(mc));
+
+        mc.onActivityDestroyed(seeded);
+        Assert.assertNull(getCurrentActivity(mc));
+    }
 }
diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/ModuleFeedbackTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleFeedbackTests.java
@@ -1,5 +1,6 @@
 package ly.count.android.sdk;
 
+import android.app.Activity;
 import androidx.annotation.NonNull;
 import androidx.test.core.app.ApplicationProvider;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
@@ -694,4 +695,90 @@ private void fillFeedbackWidgetSegmentationParams(Map<String, Object> segmentati
         segmentation.put("app_version", "1.0");
         segmentation.put("widget_id", widgetId);
     }
+
+    private Activity getCurrentActivity(ModuleFeedback module) throws Exception {
+        java.lang.reflect.Field field = ModuleFeedback.class.getDeclaredField("currentActivity");
+        field.setAccessible(true);
+        return (Activity) field.get(module);
+    }
+
+    // ======== Activity reference / leak prevention tests (issue #556) ========
+
+    /**
+     * onActivityDestroyed must null out currentActivity when the destroyed activity
+     * is the one currently tracked. This is the core leak fix.
+     */
+    @Test
+    public void onActivityDestroyed_clearsCurrentActivity_whenIdentityMatches() throws Exception {
+        ModuleFeedback mf = mCountly.moduleFeedback;
+
+        Activity act = mock(Activity.class);
+        mf.onActivityStarted(act, 1);
+        Assert.assertSame(act, getCurrentActivity(mf));
+
+        mf.onActivityDestroyed(act);
+        Assert.assertNull(getCurrentActivity(mf));
+    }
+
+    /**
+     * Destroying an activity other than the currently tracked one must NOT clear the field.
+     */
+    @Test
+    public void onActivityDestroyed_doesNotClear_whenDifferentActivity() throws Exception {
+        ModuleFeedback mf = mCountly.moduleFeedback;
+
+        Activity tracked = mock(Activity.class);
+        Activity unrelated = mock(Activity.class);
+        mf.onActivityStarted(tracked, 1);
+
+        mf.onActivityDestroyed(unrelated);
+        Assert.assertSame(tracked, getCurrentActivity(mf));
+    }
+
+    /**
+     * Rotation race regression: when onActivityStarted for the new activity fires before
+     * onActivityDestroyed for the old one, destroying the old activity must not wipe out
+     * the new tracked activity.
+     */
+    @Test
+    public void onActivityDestroyed_doesNotClearNewerActivity_afterRotationRace() throws Exception {
+        ModuleFeedback mf = mCountly.moduleFeedback;
+
+        Activity oldAct = mock(Activity.class);
+        Activity newAct = mock(Activity.class);
+
+        mf.onActivityStarted(oldAct, 1);
+        mf.onActivityStarted(newAct, 2);
+        Assert.assertSame(newAct, getCurrentActivity(mf));
+
+        mf.onActivityDestroyed(oldAct);
+        Assert.assertSame(newAct, getCurrentActivity(mf));
+    }
+
+    /**
+     * onActivityDestroyed must not throw when no activity has been tracked yet.
+     */
+    @Test
+    public void onActivityDestroyed_isSafe_whenNoActivityTracked() throws Exception {
+        ModuleFeedback mf = mCountly.moduleFeedback;
+
+        Activity stray = mock(Activity.class);
+        mf.onActivityDestroyed(stray);
+        Assert.assertNull(getCurrentActivity(mf));
+    }
+
+    /**
+     * The seeded activity path (onInitialActivitySeeded) must also be cleared on destroy.
+     */
+    @Test
+    public void onActivityDestroyed_clearsSeededActivity() throws Exception {
+        ModuleFeedback mf = mCountly.moduleFeedback;
+
+        Activity seeded = mock(Activity.class);
+        mf.onInitialActivitySeeded(seeded);
+        Assert.assertSame(seeded, getCurrentActivity(mf));
+
+        mf.onActivityDestroyed(seeded);
+        Assert.assertNull(getCurrentActivity(mf));
+    }
 }
diff --git a/sdk/src/main/java/ly/count/android/sdk/ContentOverlayView.java b/sdk/src/main/java/ly/count/android/sdk/ContentOverlayView.java
@@ -62,7 +62,10 @@ class ContentOverlayView extends FrameLayout {
         int orientation,
         @Nullable ContentCallback callback,
         @NonNull Runnable onClose) {
-        super(activity);
+        // Use Application context so View.mContext does not pin the constructing activity for
+        // the overlay's lifetime. The overlay is designed to outlive activity transitions;
+        // window attachment uses currentHostActivity (dynamically updated in attachToActivity).
+        super(activity.getApplicationContext());
 
         this.configPortrait = portrait;
         this.configLandscape = landscape;
@@ -122,9 +125,15 @@ public void onActivityStopped(@NonNull Activity a) {
 
             @Override
             public void onActivityDestroyed(@NonNull Activity a) {
-                if (a == currentHostActivity && isAddedToWindow) {
-                    Log.d(Countly.TAG, "[ContentOverlayView] onActivityDestroyed, host activity destroyed, removing from window");
-                    removeFromWindow();
+                if (a == currentHostActivity) {
+                    if (isAddedToWindow) {
+                        Log.d(Countly.TAG, "[ContentOverlayView] onActivityDestroyed, host activity destroyed, removing from window");
+                        removeFromWindow();
+                    }
+                    // Drop the strong reference to the destroyed activity so it can be GC'd.
+                    // The overlay is reattached via ModuleContent.onActivityStarted, which calls attachToActivity()
+                    // and re-sets currentHostActivity for the next host.
+                    currentHostActivity = null;
                 }
             }
         };
@@ -621,6 +630,20 @@ private void eventAction(Map<String, Object> query) {
         }
     }
 
+    private void startActivityFromOverlay(@NonNull Intent intent) {
+        // Prefer the current host activity so the launched intent stays in the same task.
+        // Fall back to Application context with NEW_TASK (mContext is App since the overlay
+        // outlives activities), which is the only legal way to start an activity from a
+        // non-activity context.
+        Activity host = currentHostActivity;
+        if (host != null && !host.isFinishing() && !host.isDestroyed()) {
+            host.startActivity(intent);
+        } else {
+            intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
+            getContext().startActivity(intent);
+        }
+    }
+
     private boolean linkAction(Map<String, Object> query, WebView view) {
         Log.i(Countly.TAG, "[ContentOverlayView] linkAction, link action detected");
         if (!query.containsKey("link")) {
@@ -632,7 +655,7 @@ private boolean linkAction(Map<String, Object> query, WebView view) {
         }
 
         Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse(link.toString()));
-        view.getContext().startActivity(intent);
+        startActivityFromOverlay(intent);
         return true;
     }
 
@@ -900,7 +923,9 @@ private void cleanupWebView() {
 
     @SuppressLint("SetJavaScriptEnabled")
     private WebView createWebView(@NonNull Activity activity, @NonNull TransparentActivityConfig config) {
-        WebView wv = new CountlyWebView(activity);
+        // Application context: WebView's mContext must not retain the constructing activity, since the overlay
+        // (and its WebView) outlives activity transitions. Activity-specific operations route through currentHostActivity.
+        WebView wv = new CountlyWebView(activity.getApplicationContext());
         wv.setVisibility(View.INVISIBLE);
         LayoutParams webLayoutParams = new LayoutParams(
             ViewGroup.LayoutParams.MATCH_PARENT, ViewGroup.LayoutParams.MATCH_PARENT);
@@ -925,8 +950,7 @@ private WebView createWebView(@NonNull Activity activity, @NonNull TransparentAc
 
             if (url.endsWith("cly_x_int=1")) {
                 Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse(url));
-                intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
-                getContext().startActivity(intent);
+                startActivityFromOverlay(intent);
                 return true;
             }
 
diff --git a/sdk/src/main/java/ly/count/android/sdk/Countly.java b/sdk/src/main/java/ly/count/android/sdk/Countly.java
@@ -783,9 +783,9 @@ public void onActivityDestroyed(Activity activity) {
                         if (L.logEnabled()) {
                             L.d("[Countly] onActivityDestroyed, " + activity.getClass().getSimpleName());
                         }
-                        //for (ModuleBase module : modules) {
-                        //    module.callbackOnActivityDestroyed(activity);
-                        //}
+                        for (ModuleBase module : modules) {
+                            module.onActivityDestroyed(activity);
+                        }
                     }
                 });
 
diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleBase.java b/sdk/src/main/java/ly/count/android/sdk/ModuleBase.java
@@ -73,6 +73,14 @@ void onInitialActivitySeeded(@NonNull Activity activity) {
     void onActivityStopped(int updatedActivityCount) {
     }
 
+    /**
+     * Called when an Activity is destroyed. Modules that hold Activity references must
+     * clear them here (using identity comparison) to prevent leaking destroyed activities
+     * through the Countly singleton.
+     */
+    void onActivityDestroyed(@NonNull Activity activity) {
+    }
+
     //void callbackOnActivityCreated(Activity activity) {
     //}
     //
diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleContent.java b/sdk/src/main/java/ly/count/android/sdk/ModuleContent.java
diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleFeedback.java b/sdk/src/main/java/ly/count/android/sdk/ModuleFeedback.java

Original file line number	Diff line number	Diff line change
`@@ -783,9 +783,9 @@ public void onActivityDestroyed(Activity activity) {`
`783`	`783`	`if (L.logEnabled()) {`
`784`	`784`	`L.d("[Countly] onActivityDestroyed, " + activity.getClass().getSimpleName());`
`785`	`785`	`}`
`786`		`- //for (ModuleBase module : modules) {`
`787`		`- // module.callbackOnActivityDestroyed(activity);`
`788`		`- //}`
	`786`	`+ for (ModuleBase module : modules) {`
	`787`	`+ module.onActivityDestroyed(activity);`
	`788`	`+ }`
`789`	`789`	`}`
`790`	`790`	`});`
`791`	`791`