Merge "camera2: Fix race conditions and deadlocks around configuration" into lmp-dev

7 files changed, 93 insertions, 21 deletions

diff --git a/core/java/android/hardware/camera2/impl/CameraCaptureSessionImpl.java b/core/java/android/hardware/camera2/impl/CameraCaptureSessionImpl.java
index b9507d722aff..68926d008d83 100644
--- a/core/java/android/hardware/camera2/impl/CameraCaptureSessionImpl.java
+++ b/core/java/android/hardware/camera2/impl/CameraCaptureSessionImpl.java
@@ -633,7 +633,11 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
         @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 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
                  *
                  * 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
 
-                // 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);
 
-                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
+                    }
 
-                    // TODO: call onError instead of onClosed if this happens
+                    mUnconfigureDrainer.beginDrain();
                 }
-
-                mUnconfigureDrainer.beginDrain();
             }
         }
     }
diff --git a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
index 257809371f4f..ec450bd14cc8 100644
--- a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
+++ b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
@@ -60,7 +60,7 @@ public class CameraDeviceImpl extends CameraDevice {
     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 e96c15f7a270..89e2d98453fd 100644
--- a/core/java/android/hardware/camera2/legacy/CameraDeviceState.java
+++ b/core/java/android/hardware/camera2/legacy/CameraDeviceState.java
@@ -73,6 +73,7 @@ public class CameraDeviceState {
         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 @@ public class CameraDeviceState {
             }
             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 c8e014783239..64c532bfb9c6 100644
--- a/core/java/android/hardware/camera2/legacy/GLThreadManager.java
+++ b/core/java/android/hardware/camera2/legacy/GLThreadManager.java
@@ -113,6 +113,9 @@ public class GLThreadManager {
                     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 4587c6fa5231..3a976ba8b862 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.graphics.SurfaceTexture;
 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 @@ public class LegacyCameraDevice implements AutoCloseable {
             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 @@ public class LegacyCameraDevice implements AutoCloseable {
 
         @Override
         public void onIdle() {
+            if (DEBUG) {
+                Log.d(TAG, "onIdle called");
+            }
+
             mIdle.open();
 
             mResultHandler.post(new Runnable() {
@@ -143,6 +166,15 @@ public class LegacyCameraDevice implements AutoCloseable {
         }
 
         @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 36cd907eb698..0699ffbc9c81 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.Looper;
 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 @@ public class RequestHandlerThread extends HandlerThread {
 
     // 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 a7ea89cab303..17ce2486b71d 100644
--- a/core/java/android/hardware/camera2/legacy/RequestThreadManager.java
+++ b/core/java/android/hardware/camera2/legacy/RequestThreadManager.java
@@ -864,6 +864,9 @@ public class RequestThreadManager {
                     }
                     resetJpegSurfaceFormats(mCallbackOutputs);
                     break;
+                case RequestHandlerThread.MSG_POKE_IDLE_HANDLER:
+                    // OK: Ignore message.
+                    break;
                 default:
                     throw new AssertionError("Unhandled message " + msg.what +
                             " on RequestThread.");