Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 99844534 authored by Yu-Ting Tseng's avatar Yu-Ting Tseng
Browse files

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
parent 54debd28

services/autofill/bugfixes.aconfig

+7 −0
Original line number Diff line number Diff line
@@ -34,3 +34,10 @@ flag { 
  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"

}

services/autofill/java/com/android/server/autofill/RemoteFillService.java

+137 −48
Original line number Diff line number Diff line
@@ -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.AbstractRemoteService; 
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 @@ final class RemoteFillService extends ServiceConnector.Impl<IAutoFillService> { 
    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 @@ final class RemoteFillService extends ServiceConnector.Impl<IAutoFillService> { 
        }

    }



    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,7 +251,8 @@ final class RemoteFillService extends ServiceConnector.Impl<IAutoFillService> { 
            }



            CompletableFuture<FillResponse> fillRequest = new CompletableFuture<>();

            remoteService.onFillCredentialRequest(request, new IFillCallback.Stub() {

            remoteService.onFillCredentialRequest(

                    request, maybeWrapWithWeakReference(new IFillCallback.Stub() {

                        @Override

                        public void onCancellable(ICancellationSignal cancellation) {

                            CompletableFuture<FillResponse> future = futureRef.get();
@@ -186,7 +274,7 @@ final class RemoteFillService extends ServiceConnector.Impl<IAutoFillService> { 
                            fillRequest.completeExceptionally(

                                    new RuntimeException(errorMessage));

                        }

            }, autofillCallback);

                    }), autofillCallback);

            return fillRequest;

        }).orTimeout(TIMEOUT_REMOTE_REQUEST_MILLIS, TimeUnit.MILLISECONDS);

        futureRef.set(connectThenFillRequest);
@@ -235,7 +323,8 @@ final class RemoteFillService extends ServiceConnector.Impl<IAutoFillService> { 
            }



            CompletableFuture<FillResponse> fillRequest = new CompletableFuture<>();

            remoteService.onFillRequest(request, new IFillCallback.Stub() {

            remoteService.onFillRequest(

                    request, maybeWrapWithWeakReference(new IFillCallback.Stub() {

                        @Override

                        public void onCancellable(ICancellationSignal cancellation) {

                            CompletableFuture<FillResponse> future = futureRef.get();
@@ -257,7 +346,7 @@ final class RemoteFillService extends ServiceConnector.Impl<IAutoFillService> { 
                            fillRequest.completeExceptionally(

                                    new RuntimeException(errorMessage));

                        }

            });

                    }));

            return fillRequest;

        }).orTimeout(TIMEOUT_REMOTE_REQUEST_MILLIS, TimeUnit.MILLISECONDS);

        futureRef.set(connectThenFillRequest);
@@ -294,7 +383,7 @@ final class RemoteFillService extends ServiceConnector.Impl<IAutoFillService> { 
            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 @@ final class RemoteFillService extends ServiceConnector.Impl<IAutoFillService> { 
                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(() -> {