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

Commit 6231472f authored by Emilian Peev's avatar Emilian Peev
Browse files

Camera: Avoid holding locks during extension initialization 

Holding the interface lock while initializing the extension
is not really necessary and can potentially result in a deadlock
in case of failure where callbacks try to close the camera capture
session in the handler thread, and another thread tries to close
the camera device acquiring the interface lock there:
Thread1:
 CameraDeviceImpl.close()
    acquire the device interface lock
    release advanced extension
        try to acquire the extension characteristics lock

Thread2:
 InitializationSessionHandler.onFailure()
    while holding the extension characteristics lock
    mCaptureSession.close()
        try to acquire the device interface lock

Additionally re-use the device interface lock for the extension session
synchronization. There is no real need to use a separate lock which
complicates the lock acquire order.

Bug: 270276341
Test: atest -c -d
cts/tests/camera/src/android/hardware/camera2/cts/CameraExtensionSessionTest.java

Change-Id: Ice808c8d5538e3adb3220d7ea51dea4b14cc2f44
parent ae41acbe

core/java/android/hardware/camera2/impl/CameraAdvancedExtensionSessionImpl.java

+57 −44
Original line number Diff line number Diff line
@@ -98,15 +98,15 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes 




    // Lock to synchronize cross-thread access to device public interface

    final Object mInterfaceLock = new Object(); // access from this class and Session only!

    final Object mInterfaceLock;



    /**

     * @hide

     */

    @RequiresPermission(android.Manifest.permission.CAMERA)

    public static CameraAdvancedExtensionSessionImpl createCameraAdvancedExtensionSession(

            @NonNull CameraDevice cameraDevice, @NonNull Context ctx,

            @NonNull ExtensionSessionConfiguration config, int sessionId)

            @NonNull android.hardware.camera2.impl.CameraDeviceImpl cameraDevice,

            @NonNull Context ctx, @NonNull ExtensionSessionConfiguration config, int sessionId)

            throws CameraAccessException, RemoteException {

        long clientId = CameraExtensionCharacteristics.registerClient(ctx);

        if (clientId < 0) {
@@ -183,7 +183,8 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes 
    }



    private CameraAdvancedExtensionSessionImpl(long extensionClientId,

            @NonNull IAdvancedExtenderImpl extender, @NonNull CameraDevice cameraDevice,

            @NonNull IAdvancedExtenderImpl extender,

            @NonNull android.hardware.camera2.impl.CameraDeviceImpl cameraDevice,

            @Nullable Surface repeatingRequestSurface, @Nullable Surface burstCaptureSurface,

            @NonNull CameraExtensionSession.StateCallback callback, @NonNull Executor executor,

            int sessionId) {
@@ -200,6 +201,7 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes 
        mInitialized = false;

        mInitializeHandler = new InitializeSessionHandler();

        mSessionId = sessionId;

        mInterfaceLock = cameraDevice.mInterfaceLock;

    }



    /**
@@ -511,6 +513,8 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes 
        public void onConfigured(@NonNull CameraCaptureSession session) {

            synchronized (mInterfaceLock) {

                mCaptureSession = session;

            }



            try {

                CameraExtensionCharacteristics.initializeSession(mInitializeHandler);

            } catch (RemoteException e) {
@@ -520,11 +524,13 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes 
            }

        }

    }

    }



    private class InitializeSessionHandler extends IInitializeSessionCallback.Stub {

        @Override

        public void onSuccess() {

            mHandler.post(new Runnable() {

                @Override

                public void run() {

                    boolean status = true;

                    synchronized (mInterfaceLock) {

                        try {
@@ -532,40 +538,47 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes 
                                mSessionProcessor.onCaptureSessionStart(mRequestProcessor);

                                mInitialized = true;

                            } else {

                        Log.v(TAG, "Failed to start capture session, session released before " +

                                "extension start!");

                                Log.v(TAG, "Failed to start capture session, session " +

                                                " released before extension start!");

                                status = false;

                        mCaptureSession.close();

                            }

                        } catch (RemoteException e) {

                            Log.e(TAG, "Failed to start capture session,"

                                    + " extension service does not respond!");

                            status = false;

                    mCaptureSession.close();

                            mInitialized = false;

                        }

                    }



                    if (status) {

                        final long ident = Binder.clearCallingIdentity();

                        try {

                    mExecutor.execute(

                            () -> mCallbacks.onConfigured(CameraAdvancedExtensionSessionImpl.this));

                            mExecutor.execute(() -> mCallbacks.onConfigured(

                                    CameraAdvancedExtensionSessionImpl.this));

                        } finally {

                            Binder.restoreCallingIdentity(ident);

                        }

                    } else {

                notifyConfigurationFailure();

                        onFailure();

                    }

                }

            });

        }



        @Override

        public void onFailure() {

            mHandler.post(new Runnable() {

                @Override

                public void run() {

                    mCaptureSession.close();



                    Log.e(TAG, "Failed to initialize proxy service session!"

                            + " This can happen when trying to configure multiple "

                            + "concurrent extension sessions!");

                    notifyConfigurationFailure();

                }

            });

        }

    }



    private final class RequestCallbackHandler extends ICaptureCallback.Stub {

core/java/android/hardware/camera2/impl/CameraExtensionSessionImpl.java

+48 −37
Original line number Diff line number Diff line
@@ -114,7 +114,7 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession { 
    private boolean mInternalRepeatingRequestEnabled = true;



    // Lock to synchronize cross-thread access to device public interface

    final Object mInterfaceLock = new Object(); // access from this class and Session only!

    final Object mInterfaceLock;



    private static int nativeGetSurfaceFormat(Surface surface) {

        return SurfaceUtils.getSurfaceFormat(surface);
@@ -125,7 +125,7 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession { 
     */

    @RequiresPermission(android.Manifest.permission.CAMERA)

    public static CameraExtensionSessionImpl createCameraExtensionSession(

            @NonNull CameraDevice cameraDevice,

            @NonNull android.hardware.camera2.impl.CameraDeviceImpl cameraDevice,

            @NonNull Context ctx,

            @NonNull ExtensionSessionConfiguration config,

            int sessionId)
@@ -223,7 +223,7 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession { 
            @NonNull IPreviewExtenderImpl previewExtender,

            @NonNull List<Size> previewSizes,

            long extensionClientId,

            @NonNull CameraDevice cameraDevice,

            @NonNull android.hardware.camera2.impl.CameraDeviceImpl cameraDevice,

            @Nullable Surface repeatingRequestSurface,

            @Nullable Surface burstCaptureSurface,

            @NonNull StateCallback callback,
@@ -249,6 +249,7 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession { 
        mSupportedRequestKeys = requestKeys;

        mSupportedResultKeys = resultKeys;

        mCaptureResultsSupported = !resultKeys.isEmpty();

        mInterfaceLock = cameraDevice.mInterfaceLock;

    }



    private void initializeRepeatingRequestPipeline() throws RemoteException {
@@ -856,6 +857,9 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession { 
    private class InitializeSessionHandler extends IInitializeSessionCallback.Stub {

        @Override

        public void onSuccess() {

            mHandler.post(new Runnable() {

                @Override

                public void run() {

                    boolean status = true;

                    ArrayList<CaptureStageImpl> initialRequestList =

                            compileInitialRequestList();
@@ -888,15 +892,22 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession { 
                        notifyConfigurationFailure();

                    }

                }

            });

        }



        @Override

        public void onFailure() {

            mHandler.post(new Runnable() {

                @Override

                public void run() {

                    mCaptureSession.close();

                    Log.e(TAG, "Failed to initialize proxy service session!"

                            + " This can happen when trying to configure multiple "

                            + "concurrent extension sessions!");

                    notifyConfigurationFailure();

                }

            });

        }

    }



    private class BurstRequestHandler extends CameraCaptureSession.CaptureCallback {