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

Commit 5532c779 authored by Sergey Volnov's avatar Sergey Volnov
Browse files

Store hard refences in a static context and pass through only weak 

references during the Content Capture Sharing.

Motivation: if the remote app is killed, we don't want a possibility of
system server holding a stroing reference (through a reference chain)
to large objects in that app. Therefore what's send in the binder has to
be a weak reference. And we will store a hard reference to those objects
in the client app's static context.

Storing hard references to objects in system_servier is less critical, because that is not going to be killed.

Bug: 148265162
Test: covered by CTS tests
Change-Id: Ie561aab6019d191cf8659fb350e045089e7781ed
(cherry picked from commit 13f65b29)
parent 810c90c7

core/java/android/service/contentcapture/ContentCaptureService.java

+73 −9
Original line number Diff line number Diff line
@@ -60,7 +60,9 @@ import com.android.internal.util.Preconditions; 
import java.io.FileDescriptor;

import java.io.PrintWriter;

import java.lang.ref.WeakReference;

import java.util.HashMap;

import java.util.List;

import java.util.Map;

import java.util.Set;

import java.util.concurrent.Executor;

import java.util.function.Consumer;
@@ -119,6 +121,9 @@ public abstract class ContentCaptureService extends Service { 
     */

    public static final String SERVICE_META_DATA = "android.content_capture";



    private final LocalDataShareAdapterResourceManager mDataShareAdapterResourceManager =

            new LocalDataShareAdapterResourceManager();



    private Handler mHandler;

    private IContentCaptureServiceCallback mCallback;
@@ -546,7 +551,8 @@ public abstract class ContentCaptureService extends Service { 
                Preconditions.checkNotNull(executor);



                DataShareReadAdapterDelegate delegate =

                        new DataShareReadAdapterDelegate(executor, adapter);

                        new DataShareReadAdapterDelegate(executor, adapter,

                                mDataShareAdapterResourceManager);



                try {

                    callback.accept(delegate);
@@ -653,16 +659,17 @@ public abstract class ContentCaptureService extends Service { 


    private static class DataShareReadAdapterDelegate extends IDataShareReadAdapter.Stub {



        private final WeakReference<LocalDataShareAdapterResourceManager> mResourceManagerReference;

        private final Object mLock = new Object();

        private final WeakReference<DataShareReadAdapter> mAdapterReference;

        private final WeakReference<Executor> mExecutorReference;



        DataShareReadAdapterDelegate(Executor executor, DataShareReadAdapter adapter) {

        DataShareReadAdapterDelegate(Executor executor, DataShareReadAdapter adapter,

                LocalDataShareAdapterResourceManager resourceManager) {

            Preconditions.checkNotNull(executor);

            Preconditions.checkNotNull(adapter);

            Preconditions.checkNotNull(resourceManager);



            mExecutorReference = new WeakReference<>(executor);

            mAdapterReference = new WeakReference<>(adapter);

            resourceManager.initializeForDelegate(this, adapter, executor);

            mResourceManagerReference = new WeakReference<>(resourceManager);

        }



        @Override
@@ -670,6 +677,10 @@ public abstract class ContentCaptureService extends Service { 
                throws RemoteException {

            synchronized (mLock) {

                executeAdapterMethodLocked(adapter -> adapter.onStart(fd), "onStart");



                // Client app and Service successfully connected, so this object would be kept alive

                // until the session has finished.

                clearHardReferences();

            }

        }
@@ -678,16 +689,23 @@ public abstract class ContentCaptureService extends Service { 
            synchronized (mLock) {

                executeAdapterMethodLocked(

                        adapter -> adapter.onError(errorCode), "onError");

                clearHardReferences();

            }

        }



        private void executeAdapterMethodLocked(Consumer<DataShareReadAdapter> adapterFn,

                String methodName) {

            DataShareReadAdapter adapter = mAdapterReference.get();

            Executor executor = mExecutorReference.get();

            LocalDataShareAdapterResourceManager resourceManager = mResourceManagerReference.get();

            if (resourceManager == null) {

                Slog.w(TAG, "Can't execute " + methodName + "(), resource manager has been GC'ed");

                return;

            }



            DataShareReadAdapter adapter = resourceManager.getAdapter(this);

            Executor executor = resourceManager.getExecutor(this);



            if (adapter == null || executor == null) {

                Slog.w(TAG, "Can't execute " + methodName + "(), references have been GC'ed");

                Slog.w(TAG, "Can't execute " + methodName + "(), references are null");

                return;

            }
@@ -698,5 +716,51 @@ public abstract class ContentCaptureService extends Service { 
                Binder.restoreCallingIdentity(identity);

            }

        }



        private void clearHardReferences() {

            LocalDataShareAdapterResourceManager resourceManager = mResourceManagerReference.get();

            if (resourceManager == null) {

                Slog.w(TAG, "Can't clear references, resource manager has been GC'ed");

                return;

            }



            resourceManager.clearHardReferences(this);

        }

    }



    /**

     * Wrapper class making sure dependencies on the current application stay in the application

     * context.

     */

    private static class LocalDataShareAdapterResourceManager {



        // Keeping hard references to the remote objects in the current process (static context)

        // to prevent them to be gc'ed during the lifetime of the application. This is an

        // artifact of only operating with weak references remotely: there has to be at least 1

        // hard reference in order for this to not be killed.

        private Map<DataShareReadAdapterDelegate, DataShareReadAdapter>

                mDataShareReadAdapterHardReferences = new HashMap<>();

        private Map<DataShareReadAdapterDelegate, Executor> mExecutorHardReferences =

                new HashMap<>();





        void initializeForDelegate(DataShareReadAdapterDelegate delegate,

                DataShareReadAdapter adapter, Executor executor) {

            mDataShareReadAdapterHardReferences.put(delegate, adapter);

            mExecutorHardReferences.remove(delegate, executor);

        }



        Executor getExecutor(DataShareReadAdapterDelegate delegate) {

            return mExecutorHardReferences.get(delegate);

        }



        DataShareReadAdapter getAdapter(DataShareReadAdapterDelegate delegate) {

            return mDataShareReadAdapterHardReferences.get(delegate);

        }



        void clearHardReferences(DataShareReadAdapterDelegate delegate) {

            mDataShareReadAdapterHardReferences.remove(delegate);

            mExecutorHardReferences.remove(delegate);

        }

    }

}

core/java/android/view/contentcapture/ContentCaptureManager.java

+75 −9
Original line number Diff line number Diff line
@@ -54,6 +54,8 @@ import java.lang.annotation.Retention; 
import java.lang.annotation.RetentionPolicy;

import java.lang.ref.WeakReference;

import java.util.ArrayList;

import java.util.HashMap;

import java.util.Map;

import java.util.Set;

import java.util.concurrent.Executor;

import java.util.function.Consumer;
@@ -360,6 +362,9 @@ public final class ContentCaptureManager { 
    @NonNull

    private final IContentCaptureManager mService;



    @GuardedBy("mLock")

    private final LocalDataShareAdapterResourceManager mDataShareAdapterResourceManager;



    @NonNull

    final ContentCaptureOptions mOptions;
@@ -399,6 +404,8 @@ public final class ContentCaptureManager { 
        // do, then we should optimize it to run the tests after the Choreographer finishes the most

        // important steps of the frame.

        mHandler = Handler.createAsync(Looper.getMainLooper());



        mDataShareAdapterResourceManager = new LocalDataShareAdapterResourceManager();

    }



    /**
@@ -681,7 +688,8 @@ public final class ContentCaptureManager { 


        try {

            mService.shareData(request,

                    new DataShareAdapterDelegate(executor, dataShareWriteAdapter));

                    new DataShareAdapterDelegate(executor, dataShareWriteAdapter,

                            mDataShareAdapterResourceManager));

        } catch (RemoteException e) {

            e.rethrowFromSystemServer();

        }
@@ -737,40 +745,53 @@ public final class ContentCaptureManager { 


    private static class DataShareAdapterDelegate extends IDataShareWriteAdapter.Stub {



        private final WeakReference<DataShareWriteAdapter> mAdapterReference;

        private final WeakReference<Executor> mExecutorReference;

        private final WeakReference<LocalDataShareAdapterResourceManager> mResourceManagerReference;



        private DataShareAdapterDelegate(Executor executor, DataShareWriteAdapter adapter) {

        private DataShareAdapterDelegate(Executor executor, DataShareWriteAdapter adapter,

                LocalDataShareAdapterResourceManager resourceManager) {

            Preconditions.checkNotNull(executor);

            Preconditions.checkNotNull(adapter);

            Preconditions.checkNotNull(resourceManager);



            mExecutorReference = new WeakReference<>(executor);

            mAdapterReference = new WeakReference<>(adapter);

            resourceManager.initializeForDelegate(this, adapter, executor);

            mResourceManagerReference = new WeakReference<>(resourceManager);

        }



        @Override

        public void write(ParcelFileDescriptor destination)

                throws RemoteException {

            executeAdapterMethodLocked(adapter -> adapter.onWrite(destination), "onWrite");



            // Client app and Service successfully connected, so this object would be kept alive

            // until the session has finished.

            clearHardReferences();

        }



        @Override

        public void error(int errorCode) throws RemoteException {

            executeAdapterMethodLocked(adapter -> adapter.onError(errorCode), "onError");

            clearHardReferences();

        }



        @Override

        public void rejected() throws RemoteException {

            executeAdapterMethodLocked(DataShareWriteAdapter::onRejected, "onRejected");

            clearHardReferences();

        }



        private void executeAdapterMethodLocked(Consumer<DataShareWriteAdapter> adapterFn,

                String methodName) {

            DataShareWriteAdapter adapter = mAdapterReference.get();

            Executor executor = mExecutorReference.get();

            LocalDataShareAdapterResourceManager resourceManager = mResourceManagerReference.get();

            if (resourceManager == null) {

                Slog.w(TAG, "Can't execute " + methodName + "(), resource manager has been GC'ed");

                return;

            }



            DataShareWriteAdapter adapter = resourceManager.getAdapter(this);

            Executor executor = resourceManager.getExecutor(this);



            if (adapter == null || executor == null) {

                Slog.w(TAG, "Can't execute " + methodName + "(), references have been GC'ed");

                Slog.w(TAG, "Can't execute " + methodName + "(), references are null");

                return;

            }
@@ -781,5 +802,50 @@ public final class ContentCaptureManager { 
                Binder.restoreCallingIdentity(identity);

            }

        }



        private void clearHardReferences() {

            LocalDataShareAdapterResourceManager resourceManager = mResourceManagerReference.get();

            if (resourceManager == null) {

                Slog.w(TAG, "Can't clear references, resource manager has been GC'ed");

                return;

            }



            resourceManager.clearHardReferences(this);

        }

    }



    /**

     * Wrapper class making sure dependencies on the current application stay in the application

     * context.

     */

    private static class LocalDataShareAdapterResourceManager {



        // Keeping hard references to the remote objects in the current process (static context)

        // to prevent them to be gc'ed during the lifetime of the application. This is an

        // artifact of only operating with weak references remotely: there has to be at least 1

        // hard reference in order for this to not be killed.

        private Map<DataShareAdapterDelegate, DataShareWriteAdapter> mWriteAdapterHardReferences =

                new HashMap<>();

        private Map<DataShareAdapterDelegate, Executor> mExecutorHardReferences =

                new HashMap<>();



        void initializeForDelegate(DataShareAdapterDelegate delegate, DataShareWriteAdapter adapter,

                Executor executor) {

            mWriteAdapterHardReferences.put(delegate, adapter);

            mExecutorHardReferences.remove(delegate, executor);

        }



        Executor getExecutor(DataShareAdapterDelegate delegate) {

            return mExecutorHardReferences.get(delegate);

        }



        DataShareWriteAdapter getAdapter(DataShareAdapterDelegate delegate) {

            return mWriteAdapterHardReferences.get(delegate);

        }



        void clearHardReferences(DataShareAdapterDelegate delegate) {

            mWriteAdapterHardReferences.remove(delegate);

            mExecutorHardReferences.remove(delegate);

        }

    }

}

services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java

+21 −57
Original line number Diff line number Diff line
@@ -89,7 +89,6 @@ import java.io.IOException; 
import java.io.InputStream;

import java.io.OutputStream;

import java.io.PrintWriter;

import java.lang.ref.WeakReference;

import java.util.ArrayList;

import java.util.HashSet;

import java.util.List;
@@ -919,35 +918,24 @@ public final class ContentCaptureManagerService extends 
    private static class DataShareCallbackDelegate extends IDataShareCallback.Stub {



        @NonNull private final DataShareRequest mDataShareRequest;

        @NonNull private final WeakReference<IDataShareWriteAdapter> mClientAdapterReference;

        @NonNull private final WeakReference<ContentCaptureManagerService> mParentServiceReference;

        @NonNull private final IDataShareWriteAdapter mClientAdapter;

        @NonNull private final ContentCaptureManagerService mParentService;



        DataShareCallbackDelegate(@NonNull DataShareRequest dataShareRequest,

                @NonNull IDataShareWriteAdapter clientAdapter,

                ContentCaptureManagerService parentService) {

            mDataShareRequest = dataShareRequest;

            mClientAdapterReference = new WeakReference<>(clientAdapter);

            mParentServiceReference = new WeakReference<>(parentService);

            mClientAdapter = clientAdapter;

            mParentService = parentService;

        }



        @Override

        public void accept(IDataShareReadAdapter serviceAdapter) throws RemoteException {

            Slog.i(TAG, "Data share request accepted by Content Capture service");



            final ContentCaptureManagerService parentService = mParentServiceReference.get();

            final IDataShareWriteAdapter clientAdapter = mClientAdapterReference.get();

            if (parentService == null || clientAdapter == null) {

                Slog.w(TAG, "Can't fulfill accept() request, because remote objects have been "

                        + "GC'ed");

                return;

            }



            final WeakReference<IDataShareReadAdapter> serviceAdapterReference =

                    new WeakReference<>(serviceAdapter);



            Pair<ParcelFileDescriptor, ParcelFileDescriptor> clientPipe = createPipe();

            if (clientPipe == null) {

                clientAdapter.error(ContentCaptureManager.DATA_SHARE_ERROR_UNKNOWN);

                mClientAdapter.error(ContentCaptureManager.DATA_SHARE_ERROR_UNKNOWN);

                serviceAdapter.error(ContentCaptureManager.DATA_SHARE_ERROR_UNKNOWN);

                return;

            }
@@ -959,7 +947,7 @@ public final class ContentCaptureManagerService extends 
            if (servicePipe == null) {

                bestEffortCloseFileDescriptors(sourceIn, sinkIn);



                clientAdapter.error(ContentCaptureManager.DATA_SHARE_ERROR_UNKNOWN);

                mClientAdapter.error(ContentCaptureManager.DATA_SHARE_ERROR_UNKNOWN);

                serviceAdapter.error(ContentCaptureManager.DATA_SHARE_ERROR_UNKNOWN);

                return;

            }
@@ -967,9 +955,9 @@ public final class ContentCaptureManagerService extends 
            ParcelFileDescriptor sourceOut = servicePipe.second;

            ParcelFileDescriptor sinkOut = servicePipe.first;



            parentService.mPackagesWithShareRequests.add(mDataShareRequest.getPackageName());

            mParentService.mPackagesWithShareRequests.add(mDataShareRequest.getPackageName());



            clientAdapter.write(sourceIn);

            mClientAdapter.write(sourceIn);

            serviceAdapter.start(sinkOut);



            // File descriptor received by the client app will be a copy of the current one. Close
@@ -977,7 +965,7 @@ public final class ContentCaptureManagerService extends 
            // current pipe.

            bestEffortCloseFileDescriptor(sourceIn);



            parentService.mDataShareExecutor.execute(() -> {

            mParentService.mDataShareExecutor.execute(() -> {

                try (InputStream fis =

                             new ParcelFileDescriptor.AutoCloseInputStream(sinkIn);

                     OutputStream fos =
@@ -996,23 +984,23 @@ public final class ContentCaptureManagerService extends 
                } catch (IOException e) {

                    Slog.e(TAG, "Failed to pipe client and service streams", e);



                    sendErrorSignal(mClientAdapterReference, serviceAdapterReference,

                    sendErrorSignal(mClientAdapter, serviceAdapter,

                            ContentCaptureManager.DATA_SHARE_ERROR_UNKNOWN);

                } finally {

                    synchronized (parentService.mLock) {

                        parentService.mPackagesWithShareRequests

                    synchronized (mParentService.mLock) {

                        mParentService.mPackagesWithShareRequests

                                .remove(mDataShareRequest.getPackageName());

                    }

                }

            });



            parentService.mHandler.postDelayed(() ->

            mParentService.mHandler.postDelayed(() ->

                    enforceDataSharingTtl(

                            sourceIn,

                            sinkIn,

                            sourceOut,

                            sinkOut,

                            serviceAdapterReference),

                            serviceAdapter),

                    MAX_DATA_SHARE_FILE_DESCRIPTORS_TTL_MS);

        }
@@ -1020,31 +1008,17 @@ public final class ContentCaptureManagerService extends 
        public void reject() throws RemoteException {

            Slog.i(TAG, "Data share request rejected by Content Capture service");



            IDataShareWriteAdapter clientAdapter = mClientAdapterReference.get();

            if (clientAdapter == null) {

                Slog.w(TAG, "Can't fulfill reject() request, because remote objects have been "

                        + "GC'ed");

                return;

            }



            clientAdapter.rejected();

            mClientAdapter.rejected();

        }



        private void enforceDataSharingTtl(ParcelFileDescriptor sourceIn,

                ParcelFileDescriptor sinkIn,

                ParcelFileDescriptor sourceOut,

                ParcelFileDescriptor sinkOut,

                WeakReference<IDataShareReadAdapter> serviceAdapterReference) {

                IDataShareReadAdapter serviceAdapter) {



            final ContentCaptureManagerService parentService = mParentServiceReference.get();

            if (parentService == null) {

                Slog.w(TAG, "Can't enforce data sharing TTL, because remote objects have been "

                        + "GC'ed");

                return;

            }



            synchronized (parentService.mLock) {

                parentService.mPackagesWithShareRequests

            synchronized (mParentService.mLock) {

                mParentService.mPackagesWithShareRequests

                        .remove(mDataShareRequest.getPackageName());



                // Interaction finished successfully <=> all data has been written to Content
@@ -1069,7 +1043,7 @@ public final class ContentCaptureManagerService extends 
                bestEffortCloseFileDescriptors(sourceIn, sinkIn, sourceOut, sinkOut);



                if (!finishedSuccessfully) {

                    sendErrorSignal(mClientAdapterReference, serviceAdapterReference,

                    sendErrorSignal(mClientAdapter, serviceAdapter,

                            ContentCaptureManager.DATA_SHARE_ERROR_TIMEOUT_INTERRUPTED);

                }

            }
@@ -1115,19 +1089,9 @@ public final class ContentCaptureManagerService extends 
        }



        private static void sendErrorSignal(

                WeakReference<IDataShareWriteAdapter> clientAdapterReference,

                WeakReference<IDataShareReadAdapter> serviceAdapterReference,

                IDataShareWriteAdapter clientAdapter,

                IDataShareReadAdapter serviceAdapter,

                int errorCode) {



            final IDataShareWriteAdapter clientAdapter = clientAdapterReference.get();

            final IDataShareReadAdapter serviceAdapter = serviceAdapterReference.get();



            if (clientAdapter == null || serviceAdapter == null) {

                Slog.w(TAG, "Can't propagate error() to read/write data share adapters, because "

                        + "remote objects have been GC'ed");

                return;

            }



            try {

                clientAdapter.error(errorCode);

            } catch (RemoteException e) {