Camera: Release extension resources after closing handler thread

To avoid possible concurrency issues, always quit the handler
thread before releasing any extension resources.
In case camera is still running, camera callbacks can still
be handled during release which can result in concurrency
exceptions.

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

Change-Id: I473a4cb04e5ea27b5c8f8221a2390219a7915e73

3 files changed, 37 insertions, 40 deletions

diff --git a/core/java/android/hardware/camera2/impl/CameraAdvancedExtensionSessionImpl.java b/core/java/android/hardware/camera2/impl/CameraAdvancedExtensionSessionImpl.java
index d5a35bc31e68..02245e49e611 100644
--- a/core/java/android/hardware/camera2/impl/CameraAdvancedExtensionSessionImpl.java
+++ b/core/java/android/hardware/camera2/impl/CameraAdvancedExtensionSessionImpl.java
@@ -446,16 +446,12 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes
         }
     }
 
-    @Override
-    protected void finalize() throws Throwable {
-        if (mHandlerThread != null) {
-            mHandlerThread.quitSafely();
-        }
-        super.finalize();
-    }
+    public void release(boolean skipCloseNotification) {
+        boolean notifyClose = false;
 
-    public void release() {
         synchronized (mInterfaceLock) {
+            mHandlerThread.quitSafely();
+
             if (mSessionProcessor != null) {
                 try {
                     mSessionProcessor.deInitSession();
@@ -469,6 +465,7 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes
             if (mExtensionClientId >= 0) {
                 CameraExtensionCharacteristics.unregisterClient(mExtensionClientId);
                 if (mInitialized) {
+                    notifyClose = true;
                     CameraExtensionCharacteristics.releaseSession();
                 }
             }
@@ -482,6 +479,16 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes
             mClientRepeatingRequestSurface = null;
             mClientCaptureSurface = null;
         }
+
+        if (notifyClose && !skipCloseNotification) {
+            final long ident = Binder.clearCallingIdentity();
+            try {
+                mExecutor.execute(() -> mCallbacks.onClosed(
+                        CameraAdvancedExtensionSessionImpl.this));
+            } finally {
+                Binder.restoreCallingIdentity(ident);
+            }
+        }
     }
 
     private void notifyConfigurationFailure() {
@@ -491,7 +498,7 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes
             }
         }
 
-        release();
+        release(true /*skipCloseNotification*/);
 
         final long ident = Binder.clearCallingIdentity();
         try {
@@ -507,15 +514,7 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes
             android.hardware.camera2.CameraCaptureSession.StateCallback {
         @Override
         public void onClosed(@NonNull CameraCaptureSession session) {
-            release();
-
-            final long ident = Binder.clearCallingIdentity();
-            try {
-                mExecutor.execute(() -> mCallbacks.onClosed(
-                       CameraAdvancedExtensionSessionImpl.this));
-            } finally {
-                Binder.restoreCallingIdentity(ident);
-            }
+            release(false /*skipCloseNotification*/);
         }
 
         @Override
diff --git a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
index 0bf812e03984..fc728a22ed5a 100644
--- a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
+++ b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
@@ -697,12 +697,12 @@ public class CameraDeviceImpl extends CameraDevice
             }
 
             if (mCurrentExtensionSession != null) {
-                mCurrentExtensionSession.release();
+                mCurrentExtensionSession.release(false /*skipCloseNotification*/);
                 mCurrentExtensionSession = null;
             }
 
             if (mCurrentAdvancedExtensionSession != null) {
-                mCurrentAdvancedExtensionSession.release();
+                mCurrentAdvancedExtensionSession.release(false /*skipCloseNotification*/);
                 mCurrentAdvancedExtensionSession = null;
             }
 
@@ -1352,12 +1352,12 @@ public class CameraDeviceImpl extends CameraDevice
             }
 
             if (mCurrentExtensionSession != null) {
-                mCurrentExtensionSession.release();
+                mCurrentExtensionSession.release(true /*skipCloseNotification*/);
                 mCurrentExtensionSession = null;
             }
 
             if (mCurrentAdvancedExtensionSession != null) {
-                mCurrentAdvancedExtensionSession.release();
+                mCurrentAdvancedExtensionSession.release(true /*skipCloseNotification*/);
                 mCurrentAdvancedExtensionSession = null;
             }
 
diff --git a/core/java/android/hardware/camera2/impl/CameraExtensionSessionImpl.java b/core/java/android/hardware/camera2/impl/CameraExtensionSessionImpl.java
index 7d29a7d275cf..ecd24914c566 100644
--- a/core/java/android/hardware/camera2/impl/CameraExtensionSessionImpl.java
+++ b/core/java/android/hardware/camera2/impl/CameraExtensionSessionImpl.java
@@ -630,18 +630,13 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession {
                 new CameraExtensionUtils.HandlerExecutor(mHandler), requestHandler);
     }
 
-    @Override
-    protected void finalize() throws Throwable {
-        if (mHandlerThread != null) {
-            mHandlerThread.quitSafely();
-        }
-        super.finalize();
-    }
-
     /** @hide */
-    public void release() {
+    public void release(boolean skipCloseNotification) {
+        boolean notifyClose = false;
+
         synchronized (mInterfaceLock) {
             mInternalRepeatingRequestEnabled = false;
+            mHandlerThread.quitSafely();
 
             try {
                 mPreviewExtender.onDeInit();
@@ -654,6 +649,7 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession {
             if (mExtensionClientId >= 0) {
                 CameraExtensionCharacteristics.unregisterClient(mExtensionClientId);
                 if (mInitialized) {
+                    notifyClose = true;
                     CameraExtensionCharacteristics.releaseSession();
                 }
             }
@@ -704,6 +700,15 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession {
             mCameraRepeatingSurface = mClientRepeatingRequestSurface = null;
             mCameraBurstSurface = mClientCaptureSurface = null;
         }
+
+        if (notifyClose && !skipCloseNotification) {
+            final long ident = Binder.clearCallingIdentity();
+            try {
+                mExecutor.execute(() -> mCallbacks.onClosed(CameraExtensionSessionImpl.this));
+            } finally {
+                Binder.restoreCallingIdentity(ident);
+            }
+        }
     }
 
     private void notifyConfigurationFailure() {
@@ -713,7 +718,7 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession {
             }
         }
 
-        release();
+        release(true /*skipCloseNotification*/);
 
         final long ident = Binder.clearCallingIdentity();
         try {
@@ -745,14 +750,7 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession {
             android.hardware.camera2.CameraCaptureSession.StateCallback {
         @Override
         public void onClosed(@NonNull CameraCaptureSession session) {
-            release();
-
-            final long ident = Binder.clearCallingIdentity();
-            try {
-                mExecutor.execute(() -> mCallbacks.onClosed(CameraExtensionSessionImpl.this));
-            } finally {
-                Binder.restoreCallingIdentity(ident);
-            }
+            release(false /*skipCloseNotification*/);
         }
 
         @Override