camera2: Fix race conditions and deadlocks around configuration
Fixes an illegal state exception that sometimes occurs during
configuration. Fixes a deadlock during unconfiguration. Fixes
the idle handler never being run during configuration.
Bug: 17628736
Change-Id: Id2c5e416f96fcbac9c718fca3cc2cf21734bc6a4
diff --git a/core/java/android/hardware/camera2/impl/CameraCaptureSessionImpl.java b/core/java/android/hardware/camera2/impl/CameraCaptureSessionImpl.java
index b9507d7..68926d0 100644
--- a/core/java/android/hardware/camera2/impl/CameraCaptureSessionImpl.java
+++ b/core/java/android/hardware/camera2/impl/CameraCaptureSessionImpl.java
@@ -633,7 +633,11 @@
@Override
public void onDrained() {
if (VERBOSE) Log.v(TAG, mIdString + "onIdleDrained");
- synchronized (CameraCaptureSessionImpl.this) {
+
+ // Take device lock before session lock so that we can call back into device
+ // without causing a deadlock
+ synchronized (mDeviceImpl.mInterfaceLock) {
+ synchronized (CameraCaptureSessionImpl.this) {
/*
* The device is now IDLE, and has settled. It will not transition to
* ACTIVE or BUSY again by itself.
@@ -642,28 +646,31 @@
*
* This operation is idempotent; a session will not be closed twice.
*/
- if (VERBOSE) Log.v(TAG, mIdString + "Session drain complete, skip unconfigure: " +
- mSkipUnconfigure);
+ if (VERBOSE)
+ Log.v(TAG, mIdString + "Session drain complete, skip unconfigure: " +
+ mSkipUnconfigure);
- // Fast path: A new capture session has replaced this one; don't unconfigure.
- if (mSkipUnconfigure) {
- mStateCallback.onClosed(CameraCaptureSessionImpl.this);
- return;
+ // Fast path: A new capture session has replaced this one; don't unconfigure.
+ if (mSkipUnconfigure) {
+ mStateCallback.onClosed(CameraCaptureSessionImpl.this);
+ return;
+ }
+
+ // Slow path: #close was called explicitly on this session; unconfigure first
+
+ try {
+ mUnconfigureDrainer.taskStarted();
+ mDeviceImpl
+ .configureOutputsChecked(null); // begin transition to unconfigured
+ } catch (CameraAccessException e) {
+ // OK: do not throw checked exceptions.
+ Log.e(TAG, mIdString + "Exception while configuring outputs: ", e);
+
+ // TODO: call onError instead of onClosed if this happens
+ }
+
+ mUnconfigureDrainer.beginDrain();
}
-
- // Slow path: #close was called explicitly on this session; unconfigure first
-
- try {
- mUnconfigureDrainer.taskStarted();
- mDeviceImpl.configureOutputsChecked(null); // begin transition to unconfigured
- } catch (CameraAccessException e) {
- // OK: do not throw checked exceptions.
- Log.e(TAG, mIdString + "Exception while configuring outputs: ", e);
-
- // TODO: call onError instead of onClosed if this happens
- }
-
- mUnconfigureDrainer.beginDrain();
}
}
}
diff --git a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
index 2578093..ec450bd1 100644
--- a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
+++ b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
@@ -60,7 +60,7 @@
private ICameraDeviceUser mRemoteDevice;
// Lock to synchronize cross-thread access to device public interface
- private final Object mInterfaceLock = new Object();
+ final Object mInterfaceLock = new Object(); // access from this class and Session only!
private final CameraDeviceCallbacks mCallbacks = new CameraDeviceCallbacks();
private final StateCallback mDeviceCallback;
diff --git a/core/java/android/hardware/camera2/legacy/CameraDeviceState.java b/core/java/android/hardware/camera2/legacy/CameraDeviceState.java
index e96c15f..89e2d98 100644
--- a/core/java/android/hardware/camera2/legacy/CameraDeviceState.java
+++ b/core/java/android/hardware/camera2/legacy/CameraDeviceState.java
@@ -73,6 +73,7 @@
void onError(int errorCode, RequestHolder holder);
void onConfiguring();
void onIdle();
+ void onBusy();
void onCaptureStarted(RequestHolder holder, long timestamp);
void onCaptureResult(CameraMetadataNative result, RequestHolder holder);
}
@@ -217,6 +218,20 @@
}
Log.i(TAG, "Legacy camera service transitioning to state " + stateName);
}
+
+ // If we transitioned into a non-IDLE/non-ERROR state then mark the device as busy
+ if(newState != STATE_ERROR && newState != STATE_IDLE) {
+ if (mCurrentState != newState && mCurrentHandler != null &&
+ mCurrentListener != null) {
+ mCurrentHandler.post(new Runnable() {
+ @Override
+ public void run() {
+ mCurrentListener.onBusy();
+ }
+ });
+ }
+ }
+
switch(newState) {
case STATE_ERROR:
if (mCurrentState != STATE_ERROR && mCurrentHandler != null &&
diff --git a/core/java/android/hardware/camera2/legacy/GLThreadManager.java b/core/java/android/hardware/camera2/legacy/GLThreadManager.java
index c8e0147..64c532b 100644
--- a/core/java/android/hardware/camera2/legacy/GLThreadManager.java
+++ b/core/java/android/hardware/camera2/legacy/GLThreadManager.java
@@ -113,6 +113,9 @@
case MSG_ALLOW_FRAMES:
mDroppingFrames = false;
break;
+ case RequestHandlerThread.MSG_POKE_IDLE_HANDLER:
+ // OK: Ignore message.
+ break;
default:
Log.e(TAG, "Unhandled message " + msg.what + " on GLThread.");
break;
diff --git a/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java b/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java
index a724b41..b7fdf43 100644
--- a/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java
+++ b/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java
@@ -21,6 +21,7 @@
import android.hardware.Camera;
import android.hardware.camera2.CameraCharacteristics;
import android.hardware.camera2.CaptureRequest;
+import android.hardware.camera2.impl.CameraDeviceImpl;
import android.hardware.camera2.impl.CaptureResultExtras;
import android.hardware.camera2.ICameraDeviceCallbacks;
import android.hardware.camera2.params.StreamConfiguration;
@@ -95,7 +96,25 @@
new CameraDeviceState.CameraDeviceStateListener() {
@Override
public void onError(final int errorCode, final RequestHolder holder) {
- mIdle.open();
+ if (DEBUG) {
+ Log.d(TAG, "onError called, errorCode = " + errorCode);
+ }
+ switch (errorCode) {
+ /*
+ * Only be considered idle if we hit a fatal error
+ * and no further requests can be processed.
+ */
+ case CameraDeviceImpl.CameraDeviceCallbacks.ERROR_CAMERA_DISCONNECTED:
+ case CameraDeviceImpl.CameraDeviceCallbacks.ERROR_CAMERA_SERVICE:
+ case CameraDeviceImpl.CameraDeviceCallbacks.ERROR_CAMERA_DEVICE: {
+ mIdle.open();
+
+ if (DEBUG) {
+ Log.d(TAG, "onError - opening idle");
+ }
+ }
+ }
+
final CaptureResultExtras extras = getExtrasFromRequest(holder);
mResultHandler.post(new Runnable() {
@Override
@@ -124,6 +143,10 @@
@Override
public void onIdle() {
+ if (DEBUG) {
+ Log.d(TAG, "onIdle called");
+ }
+
mIdle.open();
mResultHandler.post(new Runnable() {
@@ -143,6 +166,15 @@
}
@Override
+ public void onBusy() {
+ mIdle.close();
+
+ if (DEBUG) {
+ Log.d(TAG, "onBusy called");
+ }
+ }
+
+ @Override
public void onCaptureStarted(final RequestHolder holder, final long timestamp) {
final CaptureResultExtras extras = getExtrasFromRequest(holder);
diff --git a/core/java/android/hardware/camera2/legacy/RequestHandlerThread.java b/core/java/android/hardware/camera2/legacy/RequestHandlerThread.java
index 36cd907..0699ffb 100644
--- a/core/java/android/hardware/camera2/legacy/RequestHandlerThread.java
+++ b/core/java/android/hardware/camera2/legacy/RequestHandlerThread.java
@@ -23,6 +23,15 @@
import android.os.MessageQueue;
public class RequestHandlerThread extends HandlerThread {
+
+ /**
+ * Ensure that the MessageQueue's idle handler gets run by poking the message queue;
+ * normally if the message queue is already idle, the idle handler won't get invoked.
+ *
+ * <p>Users of this handler thread should ignore this message.</p>
+ */
+ public final static int MSG_POKE_IDLE_HANDLER = -1;
+
private final ConditionVariable mStarted = new ConditionVariable(false);
private final ConditionVariable mIdle = new ConditionVariable(true);
private Handler.Callback mCallback;
@@ -86,12 +95,15 @@
// Blocks until thread is idling
public void waitUntilIdle() {
- Looper looper = waitAndGetHandler().getLooper();
+ Handler handler = waitAndGetHandler();
+ Looper looper = handler.getLooper();
if (looper.isIdling()) {
return;
}
mIdle.close();
looper.getQueue().addIdleHandler(mIdleHandler);
+ // Ensure that the idle handler gets run even if the looper already went idle
+ handler.sendEmptyMessage(MSG_POKE_IDLE_HANDLER);
if (looper.isIdling()) {
return;
}
diff --git a/core/java/android/hardware/camera2/legacy/RequestThreadManager.java b/core/java/android/hardware/camera2/legacy/RequestThreadManager.java
index 788b6d8..273824e 100644
--- a/core/java/android/hardware/camera2/legacy/RequestThreadManager.java
+++ b/core/java/android/hardware/camera2/legacy/RequestThreadManager.java
@@ -822,6 +822,9 @@
mCamera.release();
}
break;
+ case RequestHandlerThread.MSG_POKE_IDLE_HANDLER:
+ // OK: Ignore message.
+ break;
default:
throw new AssertionError("Unhandled message " + msg.what +
" on RequestThread.");