From dd55b3dee558ff83f286d3abacfbd1a0c9d31278 Mon Sep 17 00:00:00 2001 From: Emilian Peev Date: Mon, 4 Oct 2021 18:27:14 -0700 Subject: [PATCH] Camera: Avoid possible deadlock when closing advanced extension session The advanced extension session request processor must not hold the 'mInterfaceLock' when calling the camera device implementation as it could result in a deadlock with the camera device close that can run at the same time and try to acquire the same locks in reverse order. The request processor will only read from 'mCameraConfigMap' which is initialized only once during session setup and will not change during the session lifetime. A sample stack trace will look like this: Service thread: 1) android.hardware.camera2.impl.CameraCaptureSessionImpl.stopRepeating 2) android.hardware.camera2.impl.CameraAdvancedExtensionSessionImpl$RequestProcessor.stopRepeating Client thread: 1) android.hardware.camera2.impl.CameraAdvancedExtensionSessionImpl.release 2) android.hardware.camera2.impl.CameraDeviceImpl.close Bug: 202074988 Test: Manual using application, Camera CTS Change-Id: If992667a7d7e54be5bea1fcc4b1d71ec1b895ca0 --- .../CameraAdvancedExtensionSessionImpl.java | 76 +++++++++---------- 1 file changed, 34 insertions(+), 42 deletions(-) diff --git a/core/java/android/hardware/camera2/impl/CameraAdvancedExtensionSessionImpl.java b/core/java/android/hardware/camera2/impl/CameraAdvancedExtensionSessionImpl.java index 8da6551f3d151..b8443fb6d14b9 100644 --- a/core/java/android/hardware/camera2/impl/CameraAdvancedExtensionSessionImpl.java +++ b/core/java/android/hardware/camera2/impl/CameraAdvancedExtensionSessionImpl.java @@ -873,21 +873,19 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes @Override public int submitBurst(List requests, IRequestCallback callback) { int seqId = -1; - synchronized (mInterfaceLock) { - try { - CaptureCallbackHandler captureCallback = new CaptureCallbackHandler(callback); - ArrayList captureRequests = new ArrayList<>(); - for (Request request : requests) { - captureRequests.add(initializeCaptureRequest(mCameraDevice, request, - mCameraConfigMap)); - } - seqId = mCaptureSession.captureBurstRequests(captureRequests, - new CameraExtensionUtils.HandlerExecutor(mHandler), captureCallback); - } catch (CameraAccessException e) { - Log.e(TAG, "Failed to submit capture requests!"); - } catch (IllegalStateException e) { - Log.e(TAG, "Capture session closed!"); + try { + CaptureCallbackHandler captureCallback = new CaptureCallbackHandler(callback); + ArrayList captureRequests = new ArrayList<>(); + for (Request request : requests) { + captureRequests.add(initializeCaptureRequest(mCameraDevice, request, + mCameraConfigMap)); } + seqId = mCaptureSession.captureBurstRequests(captureRequests, + new CameraExtensionUtils.HandlerExecutor(mHandler), captureCallback); + } catch (CameraAccessException e) { + Log.e(TAG, "Failed to submit capture requests!"); + } catch (IllegalStateException e) { + Log.e(TAG, "Capture session closed!"); } return seqId; @@ -896,18 +894,16 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes @Override public int setRepeating(Request request, IRequestCallback callback) { int seqId = -1; - synchronized (mInterfaceLock) { - try { - CaptureRequest repeatingRequest = initializeCaptureRequest(mCameraDevice, - request, mCameraConfigMap); - CaptureCallbackHandler captureCallback = new CaptureCallbackHandler(callback); - seqId = mCaptureSession.setSingleRepeatingRequest(repeatingRequest, - new CameraExtensionUtils.HandlerExecutor(mHandler), captureCallback); - } catch (CameraAccessException e) { - Log.e(TAG, "Failed to enable repeating request!"); - } catch (IllegalStateException e) { - Log.e(TAG, "Capture session closed!"); - } + try { + CaptureRequest repeatingRequest = initializeCaptureRequest(mCameraDevice, + request, mCameraConfigMap); + CaptureCallbackHandler captureCallback = new CaptureCallbackHandler(callback); + seqId = mCaptureSession.setSingleRepeatingRequest(repeatingRequest, + new CameraExtensionUtils.HandlerExecutor(mHandler), captureCallback); + } catch (CameraAccessException e) { + Log.e(TAG, "Failed to enable repeating request!"); + } catch (IllegalStateException e) { + Log.e(TAG, "Capture session closed!"); } return seqId; @@ -915,27 +911,23 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes @Override public void abortCaptures() { - synchronized (mInterfaceLock) { - try { - mCaptureSession.abortCaptures(); - } catch (CameraAccessException e) { - Log.e(TAG, "Failed during capture abort!"); - } catch (IllegalStateException e) { - Log.e(TAG, "Capture session closed!"); - } + try { + mCaptureSession.abortCaptures(); + } catch (CameraAccessException e) { + Log.e(TAG, "Failed during capture abort!"); + } catch (IllegalStateException e) { + Log.e(TAG, "Capture session closed!"); } } @Override public void stopRepeating() { - synchronized (mInterfaceLock) { - try { - mCaptureSession.stopRepeating(); - } catch (CameraAccessException e) { - Log.e(TAG, "Failed during repeating capture stop!"); - } catch (IllegalStateException e) { - Log.e(TAG, "Capture session closed!"); - } + try { + mCaptureSession.stopRepeating(); + } catch (CameraAccessException e) { + Log.e(TAG, "Failed during repeating capture stop!"); + } catch (IllegalStateException e) { + Log.e(TAG, "Capture session closed!"); } } }