From 2543bb488b2e6726b32b4a9bae0c988cc0c21ee0 Mon Sep 17 00:00:00 2001
From: Emilian Peev <epeev@google.com>
Date: Wed, 4 Aug 2021 15:38:25 -0700
Subject: [PATCH] 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
---
 .../CameraAdvancedExtensionSessionImpl.java   | 35 +++++++++----------
 .../camera2/impl/CameraDeviceImpl.java        |  8 ++---
 .../impl/CameraExtensionSessionImpl.java      | 34 +++++++++---------
 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 d5a35bc31e688..02245e49e611b 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 0bf812e03984a..fc728a22ed5a6 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 7d29a7d275cfc..ecd24914c566c 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