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
diff --git a/core/java/android/os/RemoteCallbackList.java b/core/java/android/os/RemoteCallbackList.java
index 2281fb6..b9b9a18 100644
--- a/core/java/android/os/RemoteCallbackList.java
+++ b/core/java/android/os/RemoteCallbackList.java
@@ -19,6 +19,7 @@
import android.util.ArrayMap;
import android.util.Slog;
+import java.io.PrintWriter;
import java.util.function.Consumer;
/**
@@ -399,6 +400,13 @@
}
}
+ /** @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 c123a80..15c18ac 100644
--- a/core/java/android/view/autofill/AutofillManager.java
+++ b/core/java/android/view/autofill/AutofillManager.java
@@ -51,6 +51,9 @@
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.
@@ -225,6 +228,9 @@
private IAutoFillManagerClient mServiceClient;
@GuardedBy("mLock")
+ private Cleaner mServiceClientCleaner;
+
+ @GuardedBy("mLock")
private AutofillCallback mCallback;
private final Context mContext;
@@ -958,10 +964,19 @@
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 @@
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 627afa7..7f2c080 100644
--- a/core/java/android/view/autofill/IAutoFillManager.aidl
+++ b/core/java/android/view/autofill/IAutoFillManager.aidl
@@ -32,6 +32,7 @@
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 cb91f93..e37347a 100644
--- a/services/autofill/java/com/android/server/autofill/AutofillManagerService.java
+++ b/services/autofill/java/com/android/server/autofill/AutofillManagerService.java
@@ -499,6 +499,16 @@
}
@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) {
synchronized (mLock) {
diff --git a/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java b/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java
index 751c054..5c63b90 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 @@
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 @@
}
sendStateToClients(true);
+ if (mClients != null) {
+ mClients.kill();
+ mClients = null;
+ }
}
CharSequence getServiceLabel() {
@@ -605,6 +615,9 @@
}
}
+ 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");