camera: Fix exception handling from ImageReader#detach

ImageReader has a bug where if detaching an Image from Surface fails, it
might throw a RuntimeException instead of an IllegalStateException as
stated in the javadoc. This can lead to CameraExtensionSessionImpl
crashing due to the unhandled exception. Changing ImageReader behavior
is difficult as it might break backwards compatibility.

To prevent crashes from expected exception, this CL adds exception
handling for RuntimeException when detaching images.

In addition, CameraExtensionSessionImpl caught generic `Exception` at
another place, which can lead to it consuming unintended exceptions.
This CL scopes in the exception handling to consume expected exceptions
only.

Bug: 236825255
Test: CtsCameraTestCases pass on Flame
Change-Id: I6fa81b7a19fb4017480074e33b09942aedcd2212

1 files changed, 31 insertions, 3 deletions

diff --git a/core/java/android/hardware/camera2/impl/CameraExtensionSessionImpl.java b/core/java/android/hardware/camera2/impl/CameraExtensionSessionImpl.java
index 4d0ba63d7759..336ef7ac78db 100644
--- a/core/java/android/hardware/camera2/impl/CameraExtensionSessionImpl.java
+++ b/core/java/android/hardware/camera2/impl/CameraExtensionSessionImpl.java
@@ -1739,6 +1739,20 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession {
                     // abruptly.
                     Log.w(TAG, "Output surface likely abandoned, dropping buffer!");
                     img.close();
+                } catch (RuntimeException e) {
+                    // NOTE: This is intended to catch RuntimeException from ImageReader.detachImage
+                    // ImageReader.detachImage is not supposed to throw RuntimeExceptions but the
+                    // bug went unchecked for a few years and now its behavior cannot be changed
+                    // without breaking backwards compatibility.
+
+                    if (!e.getClass().equals(RuntimeException.class)) {
+                        // re-throw any exceptions that aren't base RuntimeException since they are
+                        // coming from elsewhere, and we shouldn't silently drop those.
+                        throw e;
+                    }
+
+                    Log.w(TAG, "Output surface likely abandoned, dropping buffer!");
+                    img.close();
                 }
             }
         }
@@ -1773,9 +1787,23 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession {
                 }
                 try {
                     reader.detachImage(img);
-                } catch (Exception e) {
-                    Log.e(TAG,
-                            "Failed to detach image!");
+                } catch (IllegalStateException e) {
+                    Log.e(TAG, "Failed to detach image!");
+                    img.close();
+                    return;
+                } catch (RuntimeException e) {
+                    // NOTE: This is intended to catch RuntimeException from ImageReader.detachImage
+                    // ImageReader.detachImage is not supposed to throw RuntimeExceptions but the
+                    // bug went unchecked for a few years and now its behavior cannot be changed
+                    // without breaking backwards compatibility.
+
+                    if (!e.getClass().equals(RuntimeException.class)) {
+                        // re-throw any exceptions that aren't base RuntimeException since they are
+                        // coming from elsewhere, and we shouldn't silently drop those.
+                        throw e;
+                    }
+
+                    Log.e(TAG, "Failed to detach image!");
                     img.close();
                     return;
                 }