camera2: Fix deadlocks in shim #close and make #testInvalidCapture pass

* Also fixes configureOutputs to allow it to unconfigure
* Adds IAE checks in a few spots to validate surfaces aren't null

Bug: 15116722
Change-Id: I9ec88bccb3600eb12747d84436ead27952e87646

4 files changed, 562 insertions, 139 deletions

diff --git a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
index d9f3af4..de1d9a3 100644
--- a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
+++ b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
@@ -28,6 +28,8 @@ import android.hardware.camera2.ICameraDeviceUser;
 import android.hardware.camera2.TotalCaptureResult;
 import android.hardware.camera2.utils.CameraBinderDecorator;
 import android.hardware.camera2.utils.CameraRuntimeException;
+import android.hardware.camera2.utils.CloseableLock;
+import android.hardware.camera2.utils.CloseableLock.ScopedLock;
 import android.hardware.camera2.utils.LongParcelable;
 import android.os.Handler;
 import android.os.IBinder;
@@ -57,13 +59,14 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
     // TODO: guard every function with if (!mRemoteDevice) check (if it was closed)
     private ICameraDeviceUser mRemoteDevice;
 
-    private final Object mLock = new Object();
+    private final CloseableLock mCloseLock;
     private final CameraDeviceCallbacks mCallbacks = new CameraDeviceCallbacks();
 
     private final StateListener mDeviceListener;
     private volatile StateListener mSessionStateListener;
     private final Handler mDeviceHandler;
 
+    private volatile boolean mClosing = false;
     private boolean mInError = false;
     private boolean mIdle = true;
 
@@ -100,7 +103,9 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
     private final Runnable mCallOnOpened = new Runnable() {
         @Override
         public void run() {
-            if (!CameraDeviceImpl.this.isClosed()) {
+            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
+                if (scopedLock == null) return; // Camera already closed
+
                 StateListener sessionListener = mSessionStateListener;
                 if (sessionListener != null) {
                     sessionListener.onOpened(CameraDeviceImpl.this);
@@ -113,7 +118,9 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
     private final Runnable mCallOnUnconfigured = new Runnable() {
         @Override
         public void run() {
-            if (!CameraDeviceImpl.this.isClosed()) {
+            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
+                if (scopedLock == null) return; // Camera already closed
+
                 StateListener sessionListener = mSessionStateListener;
                 if (sessionListener != null) {
                     sessionListener.onUnconfigured(CameraDeviceImpl.this);
@@ -126,7 +133,9 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
     private final Runnable mCallOnActive = new Runnable() {
         @Override
         public void run() {
-            if (!CameraDeviceImpl.this.isClosed()) {
+            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
+                if (scopedLock == null) return; // Camera already closed
+
                 StateListener sessionListener = mSessionStateListener;
                 if (sessionListener != null) {
                     sessionListener.onActive(CameraDeviceImpl.this);
@@ -139,7 +148,9 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
     private final Runnable mCallOnBusy = new Runnable() {
         @Override
         public void run() {
-            if (!CameraDeviceImpl.this.isClosed()) {
+            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
+                if (scopedLock == null) return; // Camera already closed
+
                 StateListener sessionListener = mSessionStateListener;
                 if (sessionListener != null) {
                     sessionListener.onBusy(CameraDeviceImpl.this);
@@ -150,20 +161,29 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
     };
 
     private final Runnable mCallOnClosed = new Runnable() {
+        private boolean mClosedOnce = false;
+
         @Override
         public void run() {
+            if (mClosedOnce) {
+                throw new AssertionError("Don't post #onClosed more than once");
+            }
+
             StateListener sessionListener = mSessionStateListener;
             if (sessionListener != null) {
                 sessionListener.onClosed(CameraDeviceImpl.this);
             }
             mDeviceListener.onClosed(CameraDeviceImpl.this);
+            mClosedOnce = true;
         }
     };
 
     private final Runnable mCallOnIdle = new Runnable() {
         @Override
         public void run() {
-            if (!CameraDeviceImpl.this.isClosed()) {
+            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
+                if (scopedLock == null) return; // Camera already closed
+
                 StateListener sessionListener = mSessionStateListener;
                 if (sessionListener != null) {
                     sessionListener.onIdle(CameraDeviceImpl.this);
@@ -176,7 +196,9 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
     private final Runnable mCallOnDisconnected = new Runnable() {
         @Override
         public void run() {
-            if (!CameraDeviceImpl.this.isClosed()) {
+            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
+                if (scopedLock == null) return; // Camera already closed
+
                 StateListener sessionListener = mSessionStateListener;
                 if (sessionListener != null) {
                     sessionListener.onDisconnected(CameraDeviceImpl.this);
@@ -195,6 +217,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
         mDeviceListener = listener;
         mDeviceHandler = handler;
         mCharacteristics = characteristics;
+        mCloseLock = new CloseableLock(/*name*/"CD-" + mCameraId);
 
         final int MAX_TAG_LEN = 23;
         String tag = String.format("CameraDevice-JV-%s", mCameraId);
@@ -210,8 +233,8 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
     }
 
     public void setRemoteDevice(ICameraDeviceUser remoteDevice) {
+        try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
         // TODO: Move from decorator to direct binder-mediated exceptions
-        synchronized(mLock) {
             // If setRemoteFailure already called, do nothing
             if (mInError) return;
 
@@ -254,7 +277,9 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
         }
         final int code = failureCode;
         final boolean isError = failureIsError;
-        synchronized (mLock) {
+        try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
+            if (scopedLock == null) return; // Camera already closed, can't go to error state
+
             mInError = true;
             mDeviceHandler.post(new Runnable() {
                 @Override
@@ -280,7 +305,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
         if (outputs == null) {
             outputs = new ArrayList<Surface>();
         }
-        synchronized (mLock) {
+        try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {
             checkIfCameraClosedOrInError();
 
             HashSet<Surface> addSet = new HashSet<Surface>(outputs);    // Streams to create
@@ -344,7 +369,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
     public void createCaptureSession(List<Surface> outputs,
             CameraCaptureSession.StateListener listener, Handler handler)
             throws CameraAccessException {
-        synchronized (mLock) {
+        try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {
             if (DEBUG) {
                 Log.d(TAG, "createCaptureSession");
             }
@@ -389,7 +414,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
     @Override
     public CaptureRequest.Builder createCaptureRequest(int templateType)
             throws CameraAccessException {
-        synchronized (mLock) {
+        try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {
             checkIfCameraClosedOrInError();
 
             CameraMetadataNative templatedRequest = new CameraMetadataNative();
@@ -509,7 +534,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
             handler = checkHandler(handler);
         }
 
-        synchronized (mLock) {
+        try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {
             checkIfCameraClosedOrInError();
             int requestId;
 
@@ -581,7 +606,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
     @Override
     public void stopRepeating() throws CameraAccessException {
 
-        synchronized (mLock) {
+        try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {
             checkIfCameraClosedOrInError();
             if (mRepeatingRequestId != REQUEST_ID_NONE) {
 
@@ -612,7 +637,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
 
     private void waitUntilIdle() throws CameraAccessException {
 
-        synchronized (mLock) {
+        try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {
             checkIfCameraClosedOrInError();
             if (mRepeatingRequestId != REQUEST_ID_NONE) {
                 throw new IllegalStateException("Active repeating request ongoing");
@@ -633,7 +658,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
 
     @Override
     public void flush() throws CameraAccessException {
-        synchronized (mLock) {
+        try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {
             checkIfCameraClosedOrInError();
 
             mDeviceHandler.post(mCallOnBusy);
@@ -656,8 +681,15 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
 
     @Override
     public void close() {
-        synchronized (mLock) {
+        mClosing = true;
+        // Acquire exclusive lock, close, release (idempotent)
+        mCloseLock.close();
 
+        /*
+         *  The rest of this is safe, since no other methods will be able to execute
+         *  (they will throw ISE instead; the callbacks will get dropped)
+         */
+        {
             try {
                 if (mRemoteDevice != null) {
                     mRemoteDevice.disconnect();
@@ -805,7 +837,12 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
                 // remove request from mCaptureListenerMap
                 final int requestId = frameNumberRequestPair.getValue();
                 final CaptureListenerHolder holder;
-                synchronized (mLock) {
+                try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
+                    if (scopedLock == null) {
+                        Log.w(TAG, "Camera closed while checking sequences");
+                        return;
+                    }
+
                     int index = mCaptureListenerMap.indexOfKey(requestId);
                     holder = (index >= 0) ? mCaptureListenerMap.valueAt(index)
                             : null;
@@ -890,9 +927,12 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
         @Override
         public void onCameraError(final int errorCode, CaptureResultExtras resultExtras) {
             Runnable r = null;
-            if (isClosed()) return;
 
-            synchronized(mLock) {
+            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
+                if (scopedLock == null) {
+                    return; // Camera already closed
+                }
+
                 mInError = true;
                 switch (errorCode) {
                     case ERROR_CAMERA_DISCONNECTED:
@@ -914,25 +954,24 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
                         break;
                 }
                 CameraDeviceImpl.this.mDeviceHandler.post(r);
-            }
 
-            // Fire onCaptureSequenceCompleted
-            if (DEBUG) {
-                Log.v(TAG, String.format("got error frame %d", resultExtras.getFrameNumber()));
+                // Fire onCaptureSequenceCompleted
+                if (DEBUG) {
+                    Log.v(TAG, String.format("got error frame %d", resultExtras.getFrameNumber()));
+                }
+                mFrameNumberTracker.updateTracker(resultExtras.getFrameNumber(), /*error*/true);
+                checkAndFireSequenceComplete();
             }
-            mFrameNumberTracker.updateTracker(resultExtras.getFrameNumber(), /*error*/true);
-            checkAndFireSequenceComplete();
-
         }
 
         @Override
         public void onCameraIdle() {
-            if (isClosed()) return;
-
             if (DEBUG) {
                 Log.d(TAG, "Camera now idle");
             }
-            synchronized (mLock) {
+            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
+                if (scopedLock == null) return; // Camera already closed
+
                 if (!CameraDeviceImpl.this.mIdle) {
                     CameraDeviceImpl.this.mDeviceHandler.post(mCallOnIdle);
                 }
@@ -948,30 +987,33 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
             }
             final CaptureListenerHolder holder;
 
-            // Get the listener for this frame ID, if there is one
-            synchronized (mLock) {
+            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
+                if (scopedLock == null) return; // Camera already closed
+
+                // Get the listener for this frame ID, if there is one
                 holder = CameraDeviceImpl.this.mCaptureListenerMap.get(requestId);
-            }
 
-            if (holder == null) {
-                return;
-            }
+                if (holder == null) {
+                    return;
+                }
 
-            if (isClosed()) return;
+                if (isClosed()) return;
 
-            // Dispatch capture start notice
-            holder.getHandler().post(
-                new Runnable() {
-                    @Override
-                    public void run() {
-                        if (!CameraDeviceImpl.this.isClosed()) {
-                            holder.getListener().onCaptureStarted(
-                                CameraDeviceImpl.this,
-                                holder.getRequest(resultExtras.getSubsequenceId()),
-                                timestamp);
+                // Dispatch capture start notice
+                holder.getHandler().post(
+                    new Runnable() {
+                        @Override
+                        public void run() {
+                            if (!CameraDeviceImpl.this.isClosed()) {
+                                holder.getListener().onCaptureStarted(
+                                    CameraDeviceImpl.this,
+                                    holder.getRequest(resultExtras.getSubsequenceId()),
+                                    timestamp);
+                            }
                         }
-                    }
-                });
+                    });
+
+            }
         }
 
         @Override
@@ -984,88 +1026,91 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
                         + requestId);
             }
 
+            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
+                if (scopedLock == null) return; // Camera already closed
 
-            // TODO: Handle CameraCharacteristics access from CaptureResult correctly.
-            result.set(CameraCharacteristics.LENS_INFO_SHADING_MAP_SIZE,
-                    getCharacteristics().get(CameraCharacteristics.LENS_INFO_SHADING_MAP_SIZE));
+                // TODO: Handle CameraCharacteristics access from CaptureResult correctly.
+                result.set(CameraCharacteristics.LENS_INFO_SHADING_MAP_SIZE,
+                        getCharacteristics().get(CameraCharacteristics.LENS_INFO_SHADING_MAP_SIZE));
 
-            final CaptureListenerHolder holder;
-            synchronized (mLock) {
-                holder = CameraDeviceImpl.this.mCaptureListenerMap.get(requestId);
-            }
+                final CaptureListenerHolder holder =
+                        CameraDeviceImpl.this.mCaptureListenerMap.get(requestId);
 
-            Boolean quirkPartial = result.get(CaptureResult.QUIRKS_PARTIAL_RESULT);
-            boolean quirkIsPartialResult = (quirkPartial != null && quirkPartial);
+                Boolean quirkPartial = result.get(CaptureResult.QUIRKS_PARTIAL_RESULT);
+                boolean quirkIsPartialResult = (quirkPartial != null && quirkPartial);
 
-            // Update tracker (increment counter) when it's not a partial result.
-            if (!quirkIsPartialResult) {
-                mFrameNumberTracker.updateTracker(resultExtras.getFrameNumber(), /*error*/false);
-            }
+                // Update tracker (increment counter) when it's not a partial result.
+                if (!quirkIsPartialResult) {
+                    mFrameNumberTracker.updateTracker(resultExtras.getFrameNumber(),
+                            /*error*/false);
+                }
 
-            // Check if we have a listener for this
-            if (holder == null) {
-                if (DEBUG) {
-                    Log.d(TAG,
-                            "holder is null, early return at frame "
-                                    + resultExtras.getFrameNumber());
+                // Check if we have a listener for this
+                if (holder == null) {
+                    if (DEBUG) {
+                        Log.d(TAG,
+                                "holder is null, early return at frame "
+                                        + resultExtras.getFrameNumber());
+                    }
+                    return;
                 }
-                return;
-            }
 
-            if (isClosed()) {
-                if (DEBUG) {
-                    Log.d(TAG,
-                            "camera is closed, early return at frame "
-                                    + resultExtras.getFrameNumber());
+                if (isClosed()) {
+                    if (DEBUG) {
+                        Log.d(TAG,
+                                "camera is closed, early return at frame "
+                                        + resultExtras.getFrameNumber());
+                    }
+                    return;
                 }
-                return;
-            }
 
-            final CaptureRequest request = holder.getRequest(resultExtras.getSubsequenceId());
+                final CaptureRequest request = holder.getRequest(resultExtras.getSubsequenceId());
 
 
-            Runnable resultDispatch = null;
+                Runnable resultDispatch = null;
 
-            // Either send a partial result or the final capture completed result
-            if (quirkIsPartialResult) {
-                final CaptureResult resultAsCapture =
-                        new CaptureResult(result, request, requestId);
+                // Either send a partial result or the final capture completed result
+                if (quirkIsPartialResult) {
+                    final CaptureResult resultAsCapture =
+                            new CaptureResult(result, request, requestId);
 
-                // Partial result
-                resultDispatch = new Runnable() {
-                    @Override
-                    public void run() {
-                        if (!CameraDeviceImpl.this.isClosed()){
-                            holder.getListener().onCapturePartial(
-                                CameraDeviceImpl.this,
-                                request,
-                                resultAsCapture);
+                    // Partial result
+                    resultDispatch = new Runnable() {
+                        @Override
+                        public void run() {
+                            if (!CameraDeviceImpl.this.isClosed()){
+                                holder.getListener().onCapturePartial(
+                                    CameraDeviceImpl.this,
+                                    request,
+                                    resultAsCapture);
+                            }
                         }
-                    }
-                };
-            } else {
-                final TotalCaptureResult resultAsCapture =
-                        new TotalCaptureResult(result, request, requestId);
+                    };
+                } else {
+                    final TotalCaptureResult resultAsCapture =
+                            new TotalCaptureResult(result, request, requestId);
 
-                // Final capture result
-                resultDispatch = new Runnable() {
-                    @Override
-                    public void run() {
-                        if (!CameraDeviceImpl.this.isClosed()){
-                            holder.getListener().onCaptureCompleted(
-                                CameraDeviceImpl.this,
-                                request,
-                                resultAsCapture);
+                    // Final capture result
+                    resultDispatch = new Runnable() {
+                        @Override
+                        public void run() {
+                            if (!CameraDeviceImpl.this.isClosed()){
+                                holder.getListener().onCaptureCompleted(
+                                    CameraDeviceImpl.this,
+                                    request,
+                                    resultAsCapture);
+                            }
                         }
-                    }
-                };
-            }
+                    };
+                }
 
-            holder.getHandler().post(resultDispatch);
+                holder.getHandler().post(resultDispatch);
+
+                // Fire onCaptureSequenceCompleted
+                if (!quirkIsPartialResult) {
+                    checkAndFireSequenceComplete();
+                }
 
-            // Fire onCaptureSequenceCompleted
-            if (!quirkIsPartialResult) {
-                checkAndFireSequenceComplete();
             }
         }
 
@@ -1101,10 +1146,9 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
         }
     }
 
+    /** Whether the camera device has started to close (may not yet have finished) */
     private boolean isClosed() {
-        synchronized(mLock) {
-            return (mRemoteDevice == null);
-        }
+        return mClosing;
     }
 
     private CameraCharacteristics getCharacteristics() {
diff --git a/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java b/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java
index 3f9c6ed..50515a2 100644
--- a/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java
+++ b/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java
@@ -23,7 +23,6 @@ import android.hardware.camera2.impl.CaptureResultExtras;
 import android.hardware.camera2.ICameraDeviceCallbacks;
 import android.hardware.camera2.utils.LongParcelable;
 import android.hardware.camera2.impl.CameraMetadataNative;
-import android.hardware.camera2.utils.CameraBinderDecorator;
 import android.hardware.camera2.utils.CameraRuntimeException;
 import android.os.ConditionVariable;
 import android.os.Handler;
@@ -34,7 +33,8 @@ import android.view.Surface;
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.concurrent.atomic.AtomicInteger;
+
+import static android.hardware.camera2.utils.CameraBinderDecorator.*;
 
 /**
  * This class emulates the functionality of a Camera2 device using a the old Camera class.
@@ -54,6 +54,7 @@ public class LegacyCameraDevice implements AutoCloseable {
     private final int mCameraId;
     private final ICameraDeviceCallbacks mDeviceCallbacks;
     private final CameraDeviceState mDeviceState = new CameraDeviceState();
+    private List<Surface> mConfiguredSurfaces;
 
     private final ConditionVariable mIdle = new ConditionVariable(/*open*/true);
 
@@ -213,16 +214,35 @@ public class LegacyCameraDevice implements AutoCloseable {
     /**
      * Configure the device with a set of output surfaces.
      *
+     * <p>Using empty or {@code null} {@code outputs} is the same as unconfiguring.</p>
+     *
+     * <p>Every surface in {@code outputs} must be non-{@code null}.</p>
+     *
      * @param outputs a list of surfaces to set.
-     * @return an error code for this binder operation, or {@link CameraBinderDecorator.NO_ERROR}
+     * @return an error code for this binder operation, or {@link NO_ERROR}
      *          on success.
      */
     public int configureOutputs(List<Surface> outputs) {
+        if (outputs != null) {
+            for (Surface output : outputs) {
+                if (output == null) {
+                    Log.e(TAG, "configureOutputs - null outputs are not allowed");
+                    return BAD_VALUE;
+                }
+            }
+        }
+
         int error = mDeviceState.setConfiguring();
-        if (error == CameraBinderDecorator.NO_ERROR) {
+        if (error == NO_ERROR) {
             mRequestThreadManager.configure(outputs);
             error = mDeviceState.setIdle();
         }
+
+        // TODO: May also want to check the surfaces more deeply (e.g. state, formats, sizes..)
+        if (error == NO_ERROR) {
+            mConfiguredSurfaces = outputs != null ? new ArrayList<>(outputs) : null;
+        }
+
         return error;
     }
 
@@ -239,7 +259,35 @@ public class LegacyCameraDevice implements AutoCloseable {
      */
     public int submitRequestList(List<CaptureRequest> requestList, boolean repeating,
             /*out*/LongParcelable frameNumber) {
-        // TODO: validate request here
+        if (requestList == null || requestList.isEmpty()) {
+            Log.e(TAG, "submitRequestList - Empty/null requests are not allowed");
+            return BAD_VALUE;
+        }
+
+        // Make sure that there all requests have at least 1 surface; all surfaces are non-null
+        for (CaptureRequest request : requestList) {
+            if (request.getTargets().isEmpty()) {
+                Log.e(TAG, "submitRequestList - "
+                        + "Each request must have at least one Surface target");
+                return BAD_VALUE;
+            }
+
+            for (Surface surface : request.getTargets()) {
+                if (surface == null) {
+                    Log.e(TAG, "submitRequestList - Null Surface targets are not allowed");
+                    return BAD_VALUE;
+                } else if (mConfiguredSurfaces == null) {
+                    Log.e(TAG, "submitRequestList - must configure " +
+                            " device with valid surfaces before submitting requests");
+                    return INVALID_OPERATION;
+                } else if (!mConfiguredSurfaces.contains(surface)) {
+                    Log.e(TAG, "submitRequestList - cannot use a surface that wasn't configured");
+                    return BAD_VALUE;
+                }
+            }
+        }
+
+        // TODO: further validation of request here
         mIdle.close();
         return mRequestThreadManager.submitCaptureRequests(requestList, repeating,
                 frameNumber);
diff --git a/core/java/android/hardware/camera2/legacy/RequestThreadManager.java b/core/java/android/hardware/camera2/legacy/RequestThreadManager.java
index bf250a1..9b68e9b 100644
--- a/core/java/android/hardware/camera2/legacy/RequestThreadManager.java
+++ b/core/java/android/hardware/camera2/legacy/RequestThreadManager.java
@@ -16,7 +16,6 @@
 
 package android.hardware.camera2.legacy;
 
-import android.graphics.ImageFormat;
 import android.graphics.SurfaceTexture;
 import android.hardware.Camera;
 import android.hardware.camera2.CaptureRequest;
@@ -76,8 +75,8 @@ public class RequestThreadManager {
     private volatile RequestHolder mInFlightPreview;
     private volatile RequestHolder mInFlightJpeg;
 
-    private List<Surface> mPreviewOutputs = new ArrayList<Surface>();
-    private List<Surface> mCallbackOutputs = new ArrayList<Surface>();
+    private final List<Surface> mPreviewOutputs = new ArrayList<Surface>();
+    private final List<Surface> mCallbackOutputs = new ArrayList<Surface>();
     private GLThreadManager mGLThreadManager;
     private SurfaceTexture mPreviewTexture;
     private Camera.Parameters mParams;
@@ -315,16 +314,17 @@ public class RequestThreadManager {
         mInFlightPreview = null;
         mInFlightJpeg = null;
 
-
-        for (Surface s : outputs) {
-            int format = LegacyCameraDevice.nativeDetectSurfaceType(s);
-            switch (format) {
-                case CameraMetadataNative.NATIVE_JPEG_FORMAT:
-                    mCallbackOutputs.add(s);
-                    break;
-                default:
-                    mPreviewOutputs.add(s);
-                    break;
+        if (outputs != null) {
+            for (Surface s : outputs) {
+                int format = LegacyCameraDevice.nativeDetectSurfaceType(s);
+                switch (format) {
+                    case CameraMetadataNative.NATIVE_JPEG_FORMAT:
+                        mCallbackOutputs.add(s);
+                        break;
+                    default:
+                        mPreviewOutputs.add(s);
+                        break;
+                }
             }
         }
         mParams = mCamera.getParameters();
@@ -370,8 +370,6 @@ public class RequestThreadManager {
             }
         }
 
-
-
         // TODO: Detect and optimize single-output paths here to skip stream teeing.
         if (mGLThreadManager == null) {
             mGLThreadManager = new GLThreadManager(mCameraId);
@@ -432,7 +430,7 @@ public class RequestThreadManager {
 
     private final Handler.Callback mRequestHandlerCb = new Handler.Callback() {
         private boolean mCleanup = false;
-        private List<RequestHolder> mRepeating = null;
+        private final List<RequestHolder> mRepeating = null;
 
         @SuppressWarnings("unchecked")
         @Override
@@ -447,7 +445,8 @@ public class RequestThreadManager {
             switch (msg.what) {
                 case MSG_CONFIGURE_OUTPUTS:
                     ConfigureHolder config = (ConfigureHolder) msg.obj;
-                    Log.i(TAG, "Configure outputs: " + config.surfaces.size() +
+                    int sizes = config.surfaces != null ? config.surfaces.size() : 0;
+                    Log.i(TAG, "Configure outputs: " + sizes +
                             " surfaces configured.");
                     try {
                         configureOutputs(config.surfaces);
@@ -620,12 +619,14 @@ public class RequestThreadManager {
 
 
     /**
-     * Configure with the current output Surfaces.
+     * Configure with the current list of output Surfaces.
      *
      * <p>
      * This operation blocks until the configuration is complete.
      * </p>
      *
+     * <p>Using a {@code null} or empty {@code outputs} list is the equivalent of unconfiguring.</p>
+     *
      * @param outputs a {@link java.util.Collection} of outputs to configure.
      */
     public void configure(Collection<Surface> outputs) {
diff --git a/core/java/android/hardware/camera2/utils/CloseableLock.java b/core/java/android/hardware/camera2/utils/CloseableLock.java
new file mode 100644
index 0000000..af55055
--- /dev/null
+++ b/core/java/android/hardware/camera2/utils/CloseableLock.java
@@ -0,0 +1,330 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.hardware.camera2.utils;
+
+import android.util.Log;
+
+import java.util.concurrent.locks.Condition;
+import java.util.concurrent.locks.ReentrantLock;
+
+/**
+ * Implement a shared/exclusive lock that can be closed.
+ *
+ * <p>A shared lock can be acquired if any other shared locks are also acquired. An
+ * exclusive lock acquire will block until all shared locks have been released.</p>
+ *
+ * <p>Locks are re-entrant; trying to acquire another lock (of the same type)
+ * while a lock is already held will immediately succeed.</p>
+ *
+ * <p>Acquiring to acquire a shared lock while holding an exclusive lock or vice versa is not
+ * supported; attempting it will throw an {@link IllegalStateException}.</p>
+ *
+ * <p>If the lock is closed, all future and current acquires will immediately return {@code null}.
+ * </p>
+ */
+public class CloseableLock implements AutoCloseable {
+
+    private static final boolean VERBOSE = false;
+
+    private final String TAG = "CloseableLock";
+    private final String mName;
+
+    private volatile boolean mClosed = false;
+
+    /** If an exclusive lock is acquired by some thread. */
+    private boolean mExclusive = false;
+    /**
+     * How many shared locks are acquired by any thread:
+     *
+     * <p>Reentrant locking increments this. If an exclusive lock is held,
+     * this value will stay at 0.</p>
+     */
+    private int mSharedLocks = 0;
+
+    private final ReentrantLock mLock = new ReentrantLock();
+    /** This condition automatically releases mLock when waiting; re-acquiring it after notify */
+    private final Condition mCondition = mLock.newCondition();
+
+    /** How many times the current thread is holding the lock */
+    private final ThreadLocal<Integer> mLockCount =
+        new ThreadLocal<Integer>() {
+            @Override protected Integer initialValue() {
+                return 0;
+            }
+        };
+
+    /**
+     * Helper class to release a lock at the end of a try-with-resources statement.
+     */
+    public class ScopedLock implements AutoCloseable {
+        private ScopedLock() {}
+
+        /** Release the lock with {@link CloseableLock#releaseLock}. */
+        @Override
+        public void close() {
+            releaseLock();
+        }
+    }
+
+    /**
+     * Create a new instance; starts out with 0 locks acquired.
+     */
+    public CloseableLock() {
+        mName = "";
+    }
+
+    /**
+     * Create a new instance; starts out with 0 locks acquired.
+     *
+     * @param name set an optional name for logging functionality
+     */
+    public CloseableLock(String name) {
+        mName = name;
+    }
+
+    /**
+     * Acquires the lock exclusively (blocking), marks it as closed, then releases the lock.
+     *
+     * <p>Marking a lock as closed will fail all further acquisition attempts;
+     * it will also immediately unblock all other threads currently trying to acquire a lock.</p>
+     *
+     * <p>This operation is idempotent; calling it more than once has no effect.</p>
+     *
+     * @throws IllegalStateException
+     *          if an attempt is made to {@code close} while this thread has a lock acquired
+     */
+    @Override
+    public void close() {
+        if (mClosed) {
+            log("close - already closed; ignoring");
+            return;
+        }
+
+        ScopedLock scoper = acquireExclusiveLock();
+        // Already closed by another thread?
+        if (scoper == null) {
+            return;
+        } else if (mLockCount.get() != 1) {
+            // Future: may want to add a #releaseAndClose to allow this.
+            throw new IllegalStateException(
+                    "Cannot close while one or more acquired locks are being held by this " +
+                     "thread; release all other locks first");
+        }
+
+        try {
+            mLock.lock();
+
+            mClosed = true;
+            mExclusive = false;
+            mSharedLocks = 0;
+            mLockCount.remove();
+
+            // Notify all threads that are waiting to unblock and return immediately
+            mCondition.signalAll();
+        } finally {
+            mLock.unlock();
+        }
+
+        log("close - completed");
+    }
+
+    /**
+     * Try to acquire the lock non-exclusively, blocking until the operation completes.
+     *
+     * <p>If the lock has already been closed, or being closed before this operation returns,
+     * the call will immediately return {@code false}.</p>
+     *
+     * <p>If other threads hold a non-exclusive lock (and the lock is not yet closed),
+     * this operation will return immediately. If another thread holds an exclusive lock,
+     * this thread will block until the exclusive lock has been released.</p>
+     *
+     * <p>This lock is re-entrant; acquiring more than one non-exclusive lock per thread is
+     * supported, and must be matched by an equal number of {@link #releaseLock} calls.</p>
+     *
+     * @return {@code ScopedLock} instance if the lock was acquired, or {@code null} if the lock
+     *         was already closed.
+     *
+     * @throws IllegalStateException if this thread is already holding an exclusive lock
+     */
+    public ScopedLock acquireLock() {
+
+        int ownedLocks;
+
+        try {
+            mLock.lock();
+
+            // Lock is already closed, all further acquisitions will fail
+            if (mClosed) {
+                log("acquire lock early aborted (already closed)");
+                return null;
+            }
+
+            ownedLocks = mLockCount.get();
+
+            // This thread is already holding an exclusive lock
+            if (mExclusive && ownedLocks > 0) {
+                throw new IllegalStateException(
+                        "Cannot acquire shared lock while holding exclusive lock");
+            }
+
+            // Is another thread holding the exclusive lock? Block until we can get in.
+            while (mExclusive) {
+                mCondition.awaitUninterruptibly();
+
+                // Did another thread #close while we were waiting? Unblock immediately.
+                if (mClosed) {
+                    log("acquire lock unblocked aborted (already closed)");
+                    return null;
+                }
+            }
+
+            mSharedLocks++;
+
+            ownedLocks = mLockCount.get() + 1;
+            mLockCount.set(ownedLocks);
+        } finally {
+            mLock.unlock();
+        }
+
+        log("acquired lock (local own count = " + ownedLocks + "");
+        return new ScopedLock();
+    }
+
+    /**
+     * Try to acquire the lock exclusively, blocking until all other threads release their locks.
+     *
+     * <p>If the lock has already been closed, or being closed before this operation returns,
+     * the call will immediately return {@code false}.</p>
+     *
+     * <p>If any other threads are holding a lock, this thread will block until all
+     * other locks are released.</p>
+     *
+     * <p>This lock is re-entrant; acquiring more than one exclusive lock per thread is supported,
+     * and must be matched by an equal number of {@link #releaseLock} calls.</p>
+     *
+     * @return {@code ScopedLock} instance if the lock was acquired, or {@code null} if the lock
+     *         was already closed.
+     *
+     * @throws IllegalStateException
+     *          if an attempt is made to acquire an exclusive lock while already holding a lock
+     */
+    public ScopedLock acquireExclusiveLock() {
+
+        int ownedLocks;
+
+        try {
+            mLock.lock();
+
+            // Lock is already closed, all further acquisitions will fail
+            if (mClosed) {
+                log("acquire exclusive lock early aborted (already closed)");
+                return null;
+            }
+
+            ownedLocks = mLockCount.get();
+
+            // This thread is already holding a shared lock
+            if (!mExclusive && ownedLocks > 0) {
+                throw new IllegalStateException(
+                        "Cannot acquire exclusive lock while holding shared lock");
+            }
+
+            /*
+             * Is another thread holding the lock? Block until we can get in.
+             *
+             * If we are already holding the lock, always let it through since
+             * we are just reentering the exclusive lock.
+             */
+            while (ownedLocks == 0 && (mExclusive || mSharedLocks > 0)) {
+                mCondition.awaitUninterruptibly();
+
+             // Did another thread #close while we were waiting? Unblock immediately.
+                if (mClosed) {
+                    log("acquire exclusive lock unblocked aborted (already closed)");
+                    return null;
+                }
+            }
+
+            mExclusive = true;
+
+            ownedLocks = mLockCount.get() + 1;
+            mLockCount.set(ownedLocks);
+        } finally {
+            mLock.unlock();
+        }
+
+        log("acquired exclusive lock (local own count = " + ownedLocks + "");
+        return new ScopedLock();
+    }
+
+    /**
+     * Release a single lock that was acquired.
+     *
+     * <p>Any other other that is blocked and trying to acquire a lock will get a chance
+     * to acquire the lock.</p>
+     *
+     * @throws IllegalStateException if no locks were acquired, or if the lock was already closed
+     */
+    public void releaseLock() {
+        if (mLockCount.get() <= 0) {
+            throw new IllegalStateException(
+                    "Cannot release lock that was not acquired by this thread");
+        }
+
+        int ownedLocks;
+
+        try {
+            mLock.lock();
+
+            // Lock is already closed, it couldn't have been acquired in the first place
+            if (mClosed) {
+                throw new IllegalStateException("Do not release after the lock has been closed");
+            }
+
+            if (!mExclusive) {
+                mSharedLocks--;
+            } else {
+                if (mSharedLocks != 0) {
+                    throw new AssertionError("Too many shared locks " + mSharedLocks);
+                }
+            }
+
+            ownedLocks = mLockCount.get() - 1;
+            mLockCount.set(ownedLocks);
+
+            if (ownedLocks == 0 && mExclusive) {
+                // Wake up any threads that might be waiting for the exclusive lock to be released
+                mExclusive = false;
+                mCondition.signalAll();
+            } else if (ownedLocks == 0 && mSharedLocks == 0) {
+                // Wake up any threads that might be trying to get the exclusive lock
+                mCondition.signalAll();
+            }
+        } finally {
+            mLock.unlock();
+        }
+
+        log("released lock (local lock count " + ownedLocks + ")");
+    }
+
+    private void log(String what) {
+        if (VERBOSE) {
+            Log.v(TAG + "[" + mName + "]", what);
+        }
+    }
+
+}