From 83ede344442b67da2aee11f491af507c07083033 Mon Sep 17 00:00:00 2001
From: Rubin Xu <rubinxu@google.com>
Date: Wed, 3 Jun 2020 23:10:58 +0100
Subject: [PATCH] Kill off callback reference as soon as credential is verified

During lockscreen verification, the caller-supplied callback is
strongly-referenced by a binder object passed into system server.
As a result the callback (which can be a heavyweight object like
the activity implementing the callback) cannot be garbage-collected
until the binder object is GC'ed at the system server side, which
the caller have no control over. Mitigate this by cutting the link
between the callback and the binder object as soon as the callback
is invoked.

This helps fixing the issue that we still see password shards
in Settinng's heap, even after credentail verification has long
been completed.

Bug: 144537463
Test: manual
Change-Id: I4dce3237c5e831b2077d5e9bc6f53d2c36064f78
---
 .../internal/widget/LockPatternUtils.java     | 34 +++++++++++++++----
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/core/java/com/android/internal/widget/LockPatternUtils.java b/core/java/com/android/internal/widget/LockPatternUtils.java
index 03a7b3da6251a..93690cdfc811d 100644
--- a/core/java/com/android/internal/widget/LockPatternUtils.java
+++ b/core/java/com/android/internal/widget/LockPatternUtils.java
@@ -1430,6 +1430,32 @@ public class LockPatternUtils {
                 == StrongAuthTracker.STRONG_AUTH_REQUIRED_AFTER_USER_LOCKDOWN;
     }
 
+    private static class WrappedCallback extends ICheckCredentialProgressCallback.Stub {
+
+        private Handler mHandler;
+        private CheckCredentialProgressCallback mCallback;
+
+        WrappedCallback(Handler handler, CheckCredentialProgressCallback callback) {
+            mHandler = handler;
+            mCallback = callback;
+        }
+
+        @Override
+        public void onCredentialVerified() throws RemoteException {
+            if (mHandler == null) {
+                Log.e(TAG, "Handler is null during callback");
+            }
+            // Kill reference immediately to allow early GC at client side independent of
+            // when system_server decides to lose its reference to the
+            // ICheckCredentialProgressCallback binder object.
+            mHandler.post(() -> {
+                mCallback.onEarlyMatched();
+                mCallback = null;
+            });
+            mHandler = null;
+        }
+    }
+
     private ICheckCredentialProgressCallback wrapCallback(
             final CheckCredentialProgressCallback callback) {
         if (callback == null) {
@@ -1439,13 +1465,7 @@ public class LockPatternUtils {
                 throw new IllegalStateException("Must construct LockPatternUtils on a looper thread"
                         + " to use progress callbacks.");
             }
-            return new ICheckCredentialProgressCallback.Stub() {
-
-                @Override
-                public void onCredentialVerified() throws RemoteException {
-                    mHandler.post(callback::onEarlyMatched);
-                }
-            };
+            return new WrappedCallback(mHandler, callback);
         }
     }