Use WeakReference in RemoteFillService.
This mitigates the issue of lingering IFillCallback and ISaveCallback
stub allocations. After the change, Binder would only hold onto the new
thin callback wrappers. The actual underlying stubs would not be
prevented from being GC'ed.
Bug: 307972253
Test: atest CtsAutoFillServiceTestCases
Change-Id: I7860ad66be134a5328e8dc88e71e665406d51d7d
diff --git a/services/autofill/bugfixes.aconfig b/services/autofill/bugfixes.aconfig
index b5130a1..ced10fb 100644
--- a/services/autofill/bugfixes.aconfig
+++ b/services/autofill/bugfixes.aconfig
@@ -34,3 +34,10 @@
description: "Mitigation for autofill providers miscalculating view visibility"
bug: "291795358"
}
+
+flag {
+ name: "remote_fill_service_use_weak_reference"
+ namespace: "autofill"
+ description: "Use weak reference to address binder leak problem"
+ bug: "307972253"
+}
diff --git a/services/autofill/java/com/android/server/autofill/RemoteFillService.java b/services/autofill/java/com/android/server/autofill/RemoteFillService.java
index 6d0915b..5c93991 100644
--- a/services/autofill/java/com/android/server/autofill/RemoteFillService.java
+++ b/services/autofill/java/com/android/server/autofill/RemoteFillService.java
@@ -17,6 +17,7 @@
package com.android.server.autofill;
import static android.service.autofill.FillRequest.INVALID_REQUEST_ID;
+import static android.service.autofill.Flags.remoteFillServiceUseWeakReference;
import static com.android.server.autofill.Helper.sVerbose;
@@ -44,6 +45,7 @@
import com.android.internal.infra.ServiceConnector;
import com.android.internal.os.IResultReceiver;
+import java.lang.ref.WeakReference;
import java.util.concurrent.CancellationException;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit;
@@ -65,6 +67,8 @@
private final Object mLock = new Object();
private CompletableFuture<FillResponse> mPendingFillRequest;
private int mPendingFillRequestId = INVALID_REQUEST_ID;
+ private AtomicReference<IFillCallback> mFillCallback;
+ private AtomicReference<ISaveCallback> mSaveCallback;
private final ComponentName mComponentName;
private final boolean mIsCredentialAutofillService;
@@ -150,6 +154,89 @@
}
}
+ static class IFillCallbackDelegate extends IFillCallback.Stub {
+ private WeakReference<IFillCallback> mCallbackWeakRef;
+
+ IFillCallbackDelegate(IFillCallback callback) {
+ mCallbackWeakRef = new WeakReference(callback);
+ }
+
+ @Override
+ public void onCancellable(ICancellationSignal cancellation) throws RemoteException {
+ IFillCallback callback = mCallbackWeakRef.get();
+ if (callback != null) {
+ callback.onCancellable(cancellation);
+ }
+ }
+
+ @Override
+ public void onSuccess(FillResponse response) throws RemoteException {
+ IFillCallback callback = mCallbackWeakRef.get();
+ if (callback != null) {
+ callback.onSuccess(response);
+ }
+ }
+
+ @Override
+ public void onFailure(int requestId, CharSequence message) throws RemoteException {
+ IFillCallback callback = mCallbackWeakRef.get();
+ if (callback != null) {
+ callback.onFailure(requestId, message);
+ }
+ }
+ }
+
+ static class ISaveCallbackDelegate extends ISaveCallback.Stub {
+
+ private WeakReference<ISaveCallback> mCallbackWeakRef;
+
+ ISaveCallbackDelegate(ISaveCallback callback) {
+ mCallbackWeakRef = new WeakReference(callback);
+ }
+
+ @Override
+ public void onSuccess(IntentSender intentSender) throws RemoteException {
+ ISaveCallback callback = mCallbackWeakRef.get();
+ if (callback != null) {
+ callback.onSuccess(intentSender);
+ }
+ }
+
+ @Override
+ public void onFailure(CharSequence message) throws RemoteException {
+ ISaveCallback callback = mCallbackWeakRef.get();
+ if (callback != null) {
+ callback.onFailure(message);
+ }
+ }
+ }
+
+ /**
+ * Wraps an {@link IFillCallback} object using weak reference.
+ *
+ * This prevents lingering allocation issue by breaking the chain of strong references from
+ * Binder to {@link callback}. Since {@link callback} is not held by Binder anymore, it needs
+ * to be held by {@link mFillCallback} so it's not deallocated prematurely.
+ */
+ private IFillCallback maybeWrapWithWeakReference(IFillCallback callback) {
+ if (remoteFillServiceUseWeakReference()) {
+ mFillCallback = new AtomicReference<>(callback);
+ return new IFillCallbackDelegate(callback);
+ }
+ return callback;
+ }
+
+ /**
+ * Wraps an {@link ISaveCallback} object using weak reference.
+ */
+ private ISaveCallback maybeWrapWithWeakReference(ISaveCallback callback) {
+ if (remoteFillServiceUseWeakReference()) {
+ mSaveCallback = new AtomicReference<>(callback);
+ return new ISaveCallbackDelegate(callback);
+ }
+ return callback;
+ }
+
public void onFillCredentialRequest(@NonNull FillRequest request,
IAutoFillManagerClient autofillCallback) {
if (sVerbose) {
@@ -164,29 +251,30 @@
}
CompletableFuture<FillResponse> fillRequest = new CompletableFuture<>();
- remoteService.onFillCredentialRequest(request, new IFillCallback.Stub() {
- @Override
- public void onCancellable(ICancellationSignal cancellation) {
- CompletableFuture<FillResponse> future = futureRef.get();
- if (future != null && future.isCancelled()) {
- dispatchCancellationSignal(cancellation);
- } else {
- cancellationSink.set(cancellation);
- }
- }
+ remoteService.onFillCredentialRequest(
+ request, maybeWrapWithWeakReference(new IFillCallback.Stub() {
+ @Override
+ public void onCancellable(ICancellationSignal cancellation) {
+ CompletableFuture<FillResponse> future = futureRef.get();
+ if (future != null && future.isCancelled()) {
+ dispatchCancellationSignal(cancellation);
+ } else {
+ cancellationSink.set(cancellation);
+ }
+ }
- @Override
- public void onSuccess(FillResponse response) {
- fillRequest.complete(response);
- }
+ @Override
+ public void onSuccess(FillResponse response) {
+ fillRequest.complete(response);
+ }
- @Override
- public void onFailure(int requestId, CharSequence message) {
- String errorMessage = message == null ? "" : String.valueOf(message);
- fillRequest.completeExceptionally(
- new RuntimeException(errorMessage));
- }
- }, autofillCallback);
+ @Override
+ public void onFailure(int requestId, CharSequence message) {
+ String errorMessage = message == null ? "" : String.valueOf(message);
+ fillRequest.completeExceptionally(
+ new RuntimeException(errorMessage));
+ }
+ }), autofillCallback);
return fillRequest;
}).orTimeout(TIMEOUT_REMOTE_REQUEST_MILLIS, TimeUnit.MILLISECONDS);
futureRef.set(connectThenFillRequest);
@@ -235,29 +323,30 @@
}
CompletableFuture<FillResponse> fillRequest = new CompletableFuture<>();
- remoteService.onFillRequest(request, new IFillCallback.Stub() {
- @Override
- public void onCancellable(ICancellationSignal cancellation) {
- CompletableFuture<FillResponse> future = futureRef.get();
- if (future != null && future.isCancelled()) {
- dispatchCancellationSignal(cancellation);
- } else {
- cancellationSink.set(cancellation);
- }
- }
+ remoteService.onFillRequest(
+ request, maybeWrapWithWeakReference(new IFillCallback.Stub() {
+ @Override
+ public void onCancellable(ICancellationSignal cancellation) {
+ CompletableFuture<FillResponse> future = futureRef.get();
+ if (future != null && future.isCancelled()) {
+ dispatchCancellationSignal(cancellation);
+ } else {
+ cancellationSink.set(cancellation);
+ }
+ }
- @Override
- public void onSuccess(FillResponse response) {
- fillRequest.complete(response);
- }
+ @Override
+ public void onSuccess(FillResponse response) {
+ fillRequest.complete(response);
+ }
- @Override
- public void onFailure(int requestId, CharSequence message) {
- String errorMessage = message == null ? "" : String.valueOf(message);
- fillRequest.completeExceptionally(
- new RuntimeException(errorMessage));
- }
- });
+ @Override
+ public void onFailure(int requestId, CharSequence message) {
+ String errorMessage = message == null ? "" : String.valueOf(message);
+ fillRequest.completeExceptionally(
+ new RuntimeException(errorMessage));
+ }
+ }));
return fillRequest;
}).orTimeout(TIMEOUT_REMOTE_REQUEST_MILLIS, TimeUnit.MILLISECONDS);
futureRef.set(connectThenFillRequest);
@@ -294,7 +383,7 @@
if (sVerbose) Slog.v(TAG, "calling onSaveRequest()");
CompletableFuture<IntentSender> save = new CompletableFuture<>();
- service.onSaveRequest(request, new ISaveCallback.Stub() {
+ service.onSaveRequest(request, maybeWrapWithWeakReference(new ISaveCallback.Stub() {
@Override
public void onSuccess(IntentSender intentSender) {
save.complete(intentSender);
@@ -304,7 +393,7 @@
public void onFailure(CharSequence message) {
save.completeExceptionally(new RuntimeException(String.valueOf(message)));
}
- });
+ }));
return save;
}).orTimeout(TIMEOUT_REMOTE_REQUEST_MILLIS, TimeUnit.MILLISECONDS)
.whenComplete((res, err) -> Handler.getMain().post(() -> {