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 13f65b29745d306351c666c4f9f4a2d701513fe5)

3 files changed, 169 insertions, 75 deletions

diff --git a/core/java/android/service/contentcapture/ContentCaptureService.java b/core/java/android/service/contentcapture/ContentCaptureService.java
index 61744e423b5b..ef55f0615ec7 100644
--- a/core/java/android/service/contentcapture/ContentCaptureService.java
+++ b/core/java/android/service/contentcapture/ContentCaptureService.java
@@ -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);
+        }
     }
 }
diff --git a/core/java/android/view/contentcapture/ContentCaptureManager.java b/core/java/android/view/contentcapture/ContentCaptureManager.java
index 954b83b6f9ee..edc6b1251f2c 100644
--- a/core/java/android/view/contentcapture/ContentCaptureManager.java
+++ b/core/java/android/view/contentcapture/ContentCaptureManager.java
@@ -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);
+        }
     }
 }
diff --git a/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java b/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java
index 5d2b9f359381..ce539dabfac1 100644
--- a/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java
+++ b/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java
@@ -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) {