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
This commit is contained in:
@@ -1737,6 +1737,20 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession {
|
||||
} catch (IllegalStateException e) {
|
||||
// This is possible in case the client disconnects from the output surface
|
||||
// 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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user