BlobStore: use a separate thread for RevocableFd in system_server.

Having the callbacks on RevocableFileDescriptor coming in on the main
thread of system_server can create problems:
- system_server's main thread is heavily contended
- it can cause deadlocks: callbacks come in from vold with vold's global
  lock held; this callback needs the system_server main thread to make
  progress. But if the main thread is busy with another call into vold
  (unrelated to RevocableFd), this will result in deadlock.

Bug: 300351508
Test: atest BlobStoreManagerTest
Change-Id: Ie4c3c65bdb9303f4aaab8f76b95d3f9f133b4c3e

4 files changed, 45 insertions, 4 deletions

diff --git a/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java b/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java
index 849432653bb3..4a0d57f55086 100644
--- a/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java
+++ b/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java
@@ -536,7 +536,7 @@ class BlobMetadata {
     private ParcelFileDescriptor createRevocableFd(FileDescriptor fd,
             String callingPackage, int callingUid) throws IOException {
         final RevocableFileDescriptor revocableFd =
-                new RevocableFileDescriptor(mContext, fd);
+                new RevocableFileDescriptor(mContext, fd, BlobStoreUtils.getRevocableFdHandler());
         final Accessor accessor;
         synchronized (mRevocableFds) {
             accessor = new Accessor(callingPackage, callingUid);
diff --git a/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java b/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java
index 8eef8cebec3f..ede29ec168c0 100644
--- a/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java
+++ b/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java
@@ -223,7 +223,8 @@ class BlobStoreSession extends IBlobStoreSession.Stub {
         FileDescriptor fd = null;
         try {
             fd = openWriteInternal(offsetBytes, lengthBytes);
-            final RevocableFileDescriptor revocableFd = new RevocableFileDescriptor(mContext, fd);
+            final RevocableFileDescriptor revocableFd = new RevocableFileDescriptor(mContext, fd,
+                    BlobStoreUtils.getRevocableFdHandler());
             synchronized (mSessionLock) {
                 if (mState != STATE_OPENED) {
                     IoUtils.closeQuietly(fd);
diff --git a/apex/blobstore/service/java/com/android/server/blob/BlobStoreUtils.java b/apex/blobstore/service/java/com/android/server/blob/BlobStoreUtils.java
index 8f9427303dff..a4eae014bacd 100644
--- a/apex/blobstore/service/java/com/android/server/blob/BlobStoreUtils.java
+++ b/apex/blobstore/service/java/com/android/server/blob/BlobStoreUtils.java
@@ -24,6 +24,8 @@ import android.annotation.Nullable;
 import android.content.Context;
 import android.content.pm.PackageManager;
 import android.content.res.Resources;
+import android.os.Handler;
+import android.os.HandlerThread;
 import android.os.UserHandle;
 import android.text.format.TimeMigrationUtils;
 import android.util.Slog;
@@ -63,4 +65,27 @@ class BlobStoreUtils {
     static String formatTime(long timeMs) {
         return TimeMigrationUtils.formatMillisWithFixedFormat(timeMs);
     }
+
+    private static Handler sRevocableFdHandler;
+    private static final Object sLock = new Object();
+
+    // By default, when using a RevocableFileDescriptor, callbacks will be sent to the process'
+    // main looper. In this case that would be system_server's main looper, which is a heavily
+    // contended thread. It can also cause deadlocks, because the volume daemon 'vold' holds a lock
+    // while making these callbacks to the system_server, while at the same time the system_server
+    // main thread can make a call into vold, which requires that same vold lock. To avoid these
+    // issues, use a separate thread for the RevocableFileDescriptor's requests, so that it can
+    // make progress independently of system_server.
+    static @NonNull Handler getRevocableFdHandler() {
+        synchronized (sLock) {
+            if (sRevocableFdHandler != null) {
+                return sRevocableFdHandler;
+            }
+            final HandlerThread t = new HandlerThread("BlobFuseLooper");
+            t.start();
+            sRevocableFdHandler = new Handler(t.getLooper());
+
+            return sRevocableFdHandler;
+        }
+    }
 }
diff --git a/core/java/android/os/RevocableFileDescriptor.java b/core/java/android/os/RevocableFileDescriptor.java
index ac2cd60c81b2..7093c49fb4da 100644
--- a/core/java/android/os/RevocableFileDescriptor.java
+++ b/core/java/android/os/RevocableFileDescriptor.java
@@ -73,11 +73,26 @@ public class RevocableFileDescriptor {
         init(context, fd);
     }
 
+    public RevocableFileDescriptor(Context context, FileDescriptor fd, Handler handler)
+            throws IOException {
+        init(context, fd, handler);
+    }
+
     /** {@hide} */
     public void init(Context context, FileDescriptor fd) throws IOException {
+        init(context, fd, null);
+    }
+
+    /** {@hide} */
+    public void init(Context context, FileDescriptor fd, Handler handler) throws IOException {
         mInner = fd;
-        mOuter = context.getSystemService(StorageManager.class)
-                .openProxyFileDescriptor(ParcelFileDescriptor.MODE_READ_WRITE, mCallback);
+        StorageManager sm = context.getSystemService(StorageManager.class);
+        if (handler != null) {
+            mOuter = sm.openProxyFileDescriptor(ParcelFileDescriptor.MODE_READ_WRITE, mCallback,
+                    handler);
+        } else {
+            mOuter = sm.openProxyFileDescriptor(ParcelFileDescriptor.MODE_READ_WRITE, mCallback);
+        }
     }
 
     /**