From ccec6a6ae8b9f9785934e7d97849cbafcfcf3dcb Mon Sep 17 00:00:00 2001 From: Koji Fukui Date: Wed, 18 Oct 2017 17:48:40 +0900 Subject: [PATCH] Remove AutofillManagerClient after AutofillManager is finalized Symptom: Global reference table overflow happens on system_server. Root cause: If a process repeatedly create a context and get AutofillManager from the context, each BinderProxy of AutofillManagerClient are registered to AutofillManagerServiceImpl. They aren't removed until the process is died. BinderProxy consumes global reference and finally causes crash on system_server. Solution: Remove AutofillManagerClient from AutofillManagerServiceImpl after AutofillManager is finalized. Test: cts-tradefed run commandAndExit cts-dev -m CtsAutoFillServiceTestCases Bug: 68747744 Change-Id: I8b76c5368b54317b8696cac470cc390b36e910e7 --- core/java/android/os/RemoteCallbackList.java | 8 +++++++ .../view/autofill/AutofillManager.java | 21 ++++++++++++++++++- .../view/autofill/IAutoFillManager.aidl | 1 + .../autofill/AutofillManagerService.java | 10 +++++++++ .../autofill/AutofillManagerServiceImpl.java | 13 ++++++++++++ 5 files changed, 52 insertions(+), 1 deletion(-) diff --git a/core/java/android/os/RemoteCallbackList.java b/core/java/android/os/RemoteCallbackList.java index 2281fb6d5cc27..b9b9a18e361d9 100644 --- a/core/java/android/os/RemoteCallbackList.java +++ b/core/java/android/os/RemoteCallbackList.java @@ -19,6 +19,7 @@ package android.os; import android.util.ArrayMap; import android.util.Slog; +import java.io.PrintWriter; import java.util.function.Consumer; /** @@ -399,6 +400,13 @@ public class RemoteCallbackList { } } + /** @hide */ + public void dump(PrintWriter pw, String prefix) { + pw.print(prefix); pw.print("callbacks: "); pw.println(mCallbacks.size()); + pw.print(prefix); pw.print("killed: "); pw.println(mKilled); + pw.print(prefix); pw.print("broadcasts count: "); pw.println(mBroadcastCount); + } + private void logExcessiveCallbacks() { final long size = mCallbacks.size(); final long TOO_MANY = 3000; diff --git a/core/java/android/view/autofill/AutofillManager.java b/core/java/android/view/autofill/AutofillManager.java index c123a807800f7..15c18aca96e64 100644 --- a/core/java/android/view/autofill/AutofillManager.java +++ b/core/java/android/view/autofill/AutofillManager.java @@ -51,6 +51,9 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; +// TODO: use java.lang.ref.Cleaner once Android supports Java 9 +import sun.misc.Cleaner; + /** * The {@link AutofillManager} provides ways for apps and custom views to integrate with the * Autofill Framework lifecycle. @@ -224,6 +227,9 @@ public final class AutofillManager { @GuardedBy("mLock") private IAutoFillManagerClient mServiceClient; + @GuardedBy("mLock") + private Cleaner mServiceClientCleaner; + @GuardedBy("mLock") private AutofillCallback mCallback; @@ -958,10 +964,19 @@ public final class AutofillManager { if (mServiceClient == null) { mServiceClient = new AutofillManagerClient(this); try { - final int flags = mService.addClient(mServiceClient, mContext.getUserId()); + final int userId = mContext.getUserId(); + final int flags = mService.addClient(mServiceClient, userId); mEnabled = (flags & FLAG_ADD_CLIENT_ENABLED) != 0; sDebug = (flags & FLAG_ADD_CLIENT_DEBUG) != 0; sVerbose = (flags & FLAG_ADD_CLIENT_VERBOSE) != 0; + final IAutoFillManager service = mService; + final IAutoFillManagerClient serviceClient = mServiceClient; + mServiceClientCleaner = Cleaner.create(this, () -> { + try { + service.removeClient(serviceClient, userId); + } catch (RemoteException e) { + } + }); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } @@ -1068,6 +1083,10 @@ public final class AutofillManager { if (resetClient) { // Reset connection to system mServiceClient = null; + if (mServiceClientCleaner != null) { + mServiceClientCleaner.clean(); + mServiceClientCleaner = null; + } } } } diff --git a/core/java/android/view/autofill/IAutoFillManager.aidl b/core/java/android/view/autofill/IAutoFillManager.aidl index 627afa7f83649..7f2c080b3f096 100644 --- a/core/java/android/view/autofill/IAutoFillManager.aidl +++ b/core/java/android/view/autofill/IAutoFillManager.aidl @@ -32,6 +32,7 @@ import android.view.autofill.IAutoFillManagerClient; interface IAutoFillManager { // Returns flags: FLAG_ADD_CLIENT_ENABLED | FLAG_ADD_CLIENT_DEBUG | FLAG_ADD_CLIENT_VERBOSE int addClient(in IAutoFillManagerClient client, int userId); + void removeClient(in IAutoFillManagerClient client, int userId); int startSession(IBinder activityToken, in IBinder appCallback, in AutofillId autoFillId, in Rect bounds, in AutofillValue value, int userId, boolean hasCallback, int flags, String packageName); diff --git a/services/autofill/java/com/android/server/autofill/AutofillManagerService.java b/services/autofill/java/com/android/server/autofill/AutofillManagerService.java index cb91f9308b037..e37347a29f126 100644 --- a/services/autofill/java/com/android/server/autofill/AutofillManagerService.java +++ b/services/autofill/java/com/android/server/autofill/AutofillManagerService.java @@ -498,6 +498,16 @@ public final class AutofillManagerService extends SystemService { } } + @Override + public void removeClient(IAutoFillManagerClient client, int userId) { + synchronized (mLock) { + final AutofillManagerServiceImpl service = peekServiceForUserLocked(userId); + if (service != null) { + service.removeClientLocked(client); + } + } + } + @Override public void setAuthenticationResult(Bundle data, int sessionId, int authenticationId, int userId) { diff --git a/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java b/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java index 751c0547afd6e..5c63b90bd5355 100644 --- a/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java +++ b/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java @@ -261,6 +261,12 @@ final class AutofillManagerServiceImpl { return isEnabled(); } + void removeClientLocked(IAutoFillManagerClient client) { + if (mClients != null) { + mClients.unregister(client); + } + } + void setAuthenticationResultLocked(Bundle data, int sessionId, int authenticationId, int uid) { if (!isEnabled()) { return; @@ -478,6 +484,10 @@ final class AutofillManagerServiceImpl { } sendStateToClients(true); + if (mClients != null) { + mClients.kill(); + mClients = null; + } } CharSequence getServiceLabel() { @@ -605,6 +615,9 @@ final class AutofillManagerServiceImpl { } } + pw.print(prefix); pw.println("Clients"); + mClients.dump(pw, prefix2); + if (mEventHistory == null || mEventHistory.getEvents() == null || mEventHistory.getEvents().size() == 0) { pw.print(prefix); pw.println("No event on last fill response");