From b5a17e8e5c8187b428946ca5dfe007c854c4cbbb Mon Sep 17 00:00:00 2001
From: Ahaan Ugale <augale@google.com>
Date: Tue, 28 Jun 2022 19:24:19 +0000
Subject: [PATCH] Fix leak of RemoteAccessibilityController through
 SurfacePackage.

When a child SurfacePackage is set on a SurfaceView, the view's
RemoteAccessibilityController calls linkToDeath on the SurfacePackage's
IAccessibilityEmbeddedConnection. The death recipient -- the controller
itself -- is unlinked when the Surface is destroyed -- being notified
through a SurfaceChangedCallback registered on the ViewRootImpl.

There are 2 issues there:
* When the SurfaceView is detached, the SurfaceChangedCallback is
  unregistered, so the unlinkToDeath never happens.
* The Surface holding the SurfaceView may never be destroyed, for
  example in the case of the IME.

SurfacePackages sent over ipc seem to be used in only 2 places
currently:
* For inlined autofill suggestions in the input method. This bug causes
  a serious leak here, since the memory is held in 2 persistent
  processes -- autofill Session data in the system server (referenced
  from the SurfaceViews), and SurfaceViews in the IME.
* The Wallpaper app.

For now, this is fixed by using a WeakRef for the
RemoteAccessibilityController. The root cause will be fixed in a
followup change in a later release.

Fix: 183402294
Test: manual:
 * atest CtsAutoFillServiceTestCases:InlineLoginActivityTest\
   --iterations 5
 * Force gc: adb shell kill -10 $(adb shell pgrep ext.services mockime\
   system_server -d '\ ')
 * Rerun that a couple of times so the binder objects get cleared.
 * Check for leaks of autofill.Session in system_server and
   SurfaceViews or RemoteAccessibilityEmbeddedConnection in
   com.android.cts.mockime.
Change-Id: I5447b173313919507969872bdeb7ff3038152c23
---
 .../view/RemoteAccessibilityController.java   | 23 ++++++++++++++-----
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/core/java/android/view/RemoteAccessibilityController.java b/core/java/android/view/RemoteAccessibilityController.java
index 28b567d94e6ec..b0911d789fe14 100644
--- a/core/java/android/view/RemoteAccessibilityController.java
+++ b/core/java/android/view/RemoteAccessibilityController.java
@@ -24,6 +24,8 @@ import android.os.RemoteException;
 import android.util.Log;
 import android.view.accessibility.IAccessibilityEmbeddedConnection;
 
+import java.lang.ref.WeakReference;
+
 class RemoteAccessibilityController {
     private static final String TAG = "RemoteAccessibilityController";
     private int mHostId;
@@ -80,12 +82,17 @@ class RemoteAccessibilityController {
     /**
      * Wrapper of accessibility embedded connection for embedded view hierarchy.
      */
-    private final class RemoteAccessibilityEmbeddedConnection implements IBinder.DeathRecipient {
+    private static final class RemoteAccessibilityEmbeddedConnection
+            implements IBinder.DeathRecipient {
+        private final WeakReference<RemoteAccessibilityController> mController;
         private final IAccessibilityEmbeddedConnection mConnection;
         private final IBinder mLeashToken;
 
-        RemoteAccessibilityEmbeddedConnection(IAccessibilityEmbeddedConnection connection,
+        RemoteAccessibilityEmbeddedConnection(
+                RemoteAccessibilityController controller,
+                IAccessibilityEmbeddedConnection connection,
                 IBinder leashToken) {
+            mController = new WeakReference<>(controller);
             mConnection = connection;
             mLeashToken = leashToken;
         }
@@ -109,9 +116,13 @@ class RemoteAccessibilityController {
         @Override
         public void binderDied() {
             unlinkToDeath();
-            runOnUiThread(() -> {
-                if (mConnectionWrapper == this) {
-                    mConnectionWrapper = null;
+            RemoteAccessibilityController controller = mController.get();
+            if (controller == null) {
+                return;
+            }
+            controller.runOnUiThread(() -> {
+                if (controller.mConnectionWrapper == this) {
+                    controller.mConnectionWrapper = null;
                 }
             });
         }
@@ -128,7 +139,7 @@ class RemoteAccessibilityController {
             }
             if (connection != null && leashToken != null) {
                 mConnectionWrapper =
-                    new RemoteAccessibilityEmbeddedConnection(connection, leashToken);
+                    new RemoteAccessibilityEmbeddedConnection(this, connection, leashToken);
                 mConnectionWrapper.linkToDeath();
             }
         } catch (RemoteException e) {