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

Commit 33fdb78a authored by Martijn Coenen's avatar Martijn Coenen
Browse files

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
parent 67890dbe

apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java

+1 −1
Original line number Diff line number Diff line
@@ -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);

apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java

+2 −1
Original line number Diff line number Diff line
@@ -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);

apex/blobstore/service/java/com/android/server/blob/BlobStoreUtils.java

+25 −0
Original line number Diff line number Diff line
@@ -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;

        }

    }

}

core/java/android/os/RevocableFileDescriptor.java

+17 −2
Original line number Diff line number Diff line
@@ -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);

        }

    }



    /**