From c9f0c232ae6247af96a38776965c3c7a5d27bef2 Mon Sep 17 00:00:00 2001
From: TYM Tsai <tymtsai@google.com>
Date: Fri, 19 Jun 2020 10:38:02 +0800
Subject: [PATCH] Fix the context leak in the ContentCapture

Activity cannot be released because the context be holded by multiple
objects.
* Activity holds ContentCaptureManager.
* ContentCaptureManager holds the context and MainContextCaptureSession.
* MainContextCaptureSession holds the context and ContentCaptureManager.
* The system server holds some binder references to MainContentCaptureSession.
If the system service never released the binder references, then the
activity is also never GC'd.
To avoid the issue,
1. Make the session state receiver of MainContextCaptureSession to be
   static and uses weak reference to MainContextCaptureSession.
2. The direct service vulture may miss to do unlinkToDeath(), do a checking
   while the session destory.

Bug: 143210612
Test: manual check Objects on the meminfo
Test: Activities should be released after a period of time
Test: adb shell dumpsys meminfo com.google.android.dialer
Change-Id: I12037483addb1efe444c74fa189ef6afd15821dd
---
 .../MainContentCaptureSession.java            | 72 ++++++++++++-------
 1 file changed, 45 insertions(+), 27 deletions(-)

diff --git a/core/java/android/view/contentcapture/MainContentCaptureSession.java b/core/java/android/view/contentcapture/MainContentCaptureSession.java
index 6eb71f747be6f..c43beea98c7e2 100644
--- a/core/java/android/view/contentcapture/MainContentCaptureSession.java
+++ b/core/java/android/view/contentcapture/MainContentCaptureSession.java
@@ -52,6 +52,7 @@ import android.view.contentcapture.ViewNode.ViewStructureImpl;
 import com.android.internal.os.IResultReceiver;
 
 import java.io.PrintWriter;
+import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -146,7 +147,44 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
      * Binder object used to update the session state.
      */
     @NonNull
-    private final IResultReceiver.Stub mSessionStateReceiver;
+    private final SessionStateReceiver mSessionStateReceiver;
+
+    private static class SessionStateReceiver extends IResultReceiver.Stub {
+        private final WeakReference<MainContentCaptureSession> mMainSession;
+
+        SessionStateReceiver(MainContentCaptureSession session) {
+            mMainSession = new WeakReference<>(session);
+        }
+
+        @Override
+        public void send(int resultCode, Bundle resultData) {
+            final MainContentCaptureSession mainSession = mMainSession.get();
+            if (mainSession == null) {
+                Log.w(TAG, "received result after mina session released");
+                return;
+            }
+            final IBinder binder;
+            if (resultData != null) {
+                // Change in content capture enabled.
+                final boolean hasEnabled = resultData.getBoolean(EXTRA_ENABLED_STATE);
+                if (hasEnabled) {
+                    final boolean disabled = (resultCode == RESULT_CODE_FALSE);
+                    mainSession.mDisabled.set(disabled);
+                    return;
+                }
+                binder = resultData.getBinder(EXTRA_BINDER);
+                if (binder == null) {
+                    Log.wtf(TAG, "No " + EXTRA_BINDER + " extra result");
+                    mainSession.mHandler.post(() -> mainSession.resetSession(
+                            STATE_DISABLED | STATE_INTERNAL_ERROR));
+                    return;
+                }
+            } else {
+                binder = null;
+            }
+            mainSession.mHandler.post(() -> mainSession.onSessionStarted(resultCode, binder));
+        }
+    }
 
     protected MainContentCaptureSession(@NonNull Context context,
             @NonNull ContentCaptureManager manager, @NonNull Handler handler,
@@ -159,32 +197,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
         final int logHistorySize = mManager.mOptions.logHistorySize;
         mFlushHistory = logHistorySize > 0 ? new LocalLog(logHistorySize) : null;
 
-        mSessionStateReceiver = new IResultReceiver.Stub() {
-            @Override
-            public void send(int resultCode, Bundle resultData) {
-                final IBinder binder;
-                if (resultData != null) {
-                    // Change in content capture enabled.
-                    final boolean hasEnabled = resultData.getBoolean(EXTRA_ENABLED_STATE);
-                    if (hasEnabled) {
-                        final boolean disabled = (resultCode == RESULT_CODE_FALSE);
-                        mDisabled.set(disabled);
-                        return;
-                    }
-                    binder = resultData.getBinder(EXTRA_BINDER);
-                    if (binder == null) {
-                        Log.wtf(TAG, "No " + EXTRA_BINDER + " extra result");
-                        mHandler.post(() -> resetSession(
-                                STATE_DISABLED | STATE_INTERNAL_ERROR));
-                        return;
-                    }
-                } else {
-                    binder = null;
-                }
-                mHandler.post(() -> onSessionStarted(resultCode, binder));
-            }
-        };
-
+        mSessionStateReceiver = new SessionStateReceiver(this);
     }
 
     @Override
@@ -543,6 +556,11 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
             Log.e(TAG, "Error destroying system-service session " + mId + " for "
                     + getDebugState() + ": " + e);
         }
+
+        if (mDirectServiceInterface != null) {
+            mDirectServiceInterface.asBinder().unlinkToDeath(mDirectServiceVulture, 0);
+        }
+        mDirectServiceInterface = null;
     }
 
     // TODO(b/122454205): once we support multiple sessions, we might need to move some of these