From 9e8348b803fa0aa261b3be4845b25a43bbf981d3 Mon Sep 17 00:00:00 2001 From: Emilian Peev Date: Fri, 21 Apr 2023 14:42:58 -0700 Subject: [PATCH] Camera: Quit extension handler thread during release The extension framework logic assumes that there will be no further handler loop processing after the release method quits the corresponding thread. The current approach does allow pending messages to be handled which depending on timing can result in data races. To avoid potential data races ensure that the handler thread quits regardless of any queued messages. Additionally synchronize access to 'mImageListenerMap' in 'CameraOutputImageCallback'. Bug: 278160240 Test: Camera CTS Change-Id: I3a78fa566cfe72dbfa1bc5721854823243e03f23 --- .../impl/CameraExtensionSessionImpl.java | 132 ++++++++++-------- 1 file changed, 71 insertions(+), 61 deletions(-) diff --git a/core/java/android/hardware/camera2/impl/CameraExtensionSessionImpl.java b/core/java/android/hardware/camera2/impl/CameraExtensionSessionImpl.java index 8e7c7e0cfca89..0bb5046e21df1 100644 --- a/core/java/android/hardware/camera2/impl/CameraExtensionSessionImpl.java +++ b/core/java/android/hardware/camera2/impl/CameraExtensionSessionImpl.java @@ -828,7 +828,7 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession { synchronized (mInterfaceLock) { mInternalRepeatingRequestEnabled = false; - mHandlerThread.quitSafely(); + mHandlerThread.quit(); try { mPreviewExtender.onDeInit(); @@ -1368,88 +1368,98 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession { @Override public void onImageAvailable(ImageReader reader) { Image img; - try { - img = reader.acquireNextImage(); - } catch (IllegalStateException e) { - Log.e(TAG, "Failed to acquire image, too many images pending!"); - mOutOfBuffers = true; - return; - } - if (img == null) { - Log.e(TAG, "Invalid image!"); - return; - } - - Long timestamp = img.getTimestamp(); - if (mImageListenerMap.containsKey(timestamp)) { - Pair entry = mImageListenerMap.remove(timestamp); - if (entry.second != null) { - entry.second.onImageAvailable(reader, img); - } else { - Log.w(TAG, "Invalid image listener, dropping frame!"); - img.close(); + synchronized (mInterfaceLock) { + try { + img = reader.acquireNextImage(); + } catch (IllegalStateException e) { + Log.e(TAG, "Failed to acquire image, too many images pending!"); + mOutOfBuffers = true; + return; + } + if (img == null) { + Log.e(TAG, "Invalid image!"); + return; } - } else { - mImageListenerMap.put(img.getTimestamp(), new Pair<>(img, null)); - } - notifyDroppedImages(timestamp); + Long timestamp = img.getTimestamp(); + if (mImageListenerMap.containsKey(timestamp)) { + Pair entry = mImageListenerMap.remove( + timestamp); + if (entry.second != null) { + entry.second.onImageAvailable(reader, img); + } else { + Log.w(TAG, "Invalid image listener, dropping frame!"); + img.close(); + } + } else { + mImageListenerMap.put(timestamp, new Pair<>(img, null)); + } + + notifyDroppedImages(timestamp); + } } private void notifyDroppedImages(long timestamp) { - Set timestamps = mImageListenerMap.keySet(); - ArrayList removedTs = new ArrayList<>(); - for (long ts : timestamps) { - if (ts < timestamp) { - Log.e(TAG, "Dropped image with ts: " + ts); - Pair entry = mImageListenerMap.get(ts); - if (entry.second != null) { - entry.second.onImageDropped(ts); + synchronized (mInterfaceLock) { + Set timestamps = mImageListenerMap.keySet(); + ArrayList removedTs = new ArrayList<>(); + for (long ts : timestamps) { + if (ts < timestamp) { + Log.e(TAG, "Dropped image with ts: " + ts); + Pair entry = mImageListenerMap.get(ts); + if (entry.second != null) { + entry.second.onImageDropped(ts); + } + if (entry.first != null) { + entry.first.close(); + } + removedTs.add(ts); } - if (entry.first != null) { - entry.first.close(); - } - removedTs.add(ts); } - } - for (long ts : removedTs) { - mImageListenerMap.remove(ts); + for (long ts : removedTs) { + mImageListenerMap.remove(ts); + } } } public void registerListener(Long timestamp, OnImageAvailableListener listener) { - if (mImageListenerMap.containsKey(timestamp)) { - Pair entry = mImageListenerMap.remove(timestamp); - if (entry.first != null) { - listener.onImageAvailable(mImageReader, entry.first); - if (mOutOfBuffers) { - mOutOfBuffers = false; - Log.w(TAG,"Out of buffers, retry!"); - onImageAvailable(mImageReader); + synchronized (mInterfaceLock) { + if (mImageListenerMap.containsKey(timestamp)) { + Pair entry = mImageListenerMap.remove( + timestamp); + if (entry.first != null) { + listener.onImageAvailable(mImageReader, entry.first); + if (mOutOfBuffers) { + mOutOfBuffers = false; + Log.w(TAG,"Out of buffers, retry!"); + onImageAvailable(mImageReader); + } + } else { + Log.w(TAG, "No valid image for listener with ts: " + + timestamp.longValue()); } } else { - Log.w(TAG, "No valid image for listener with ts: " + - timestamp.longValue()); + mImageListenerMap.put(timestamp, new Pair<>(null, listener)); } - } else { - mImageListenerMap.put(timestamp, new Pair<>(null, listener)); } } @Override public void close() { - for (Pair entry : mImageListenerMap.values()) { - if (entry.first != null) { - entry.first.close(); + synchronized (mInterfaceLock) { + for (Pair entry : mImageListenerMap.values()) { + if (entry.first != null) { + entry.first.close(); + } } - } - for (long timestamp : mImageListenerMap.keySet()) { - Pair entry = mImageListenerMap.get(timestamp); - if (entry.second != null) { - entry.second.onImageDropped(timestamp); + for (long timestamp : mImageListenerMap.keySet()) { + Pair entry = mImageListenerMap.get(timestamp); + if (entry.second != null) { + entry.second.onImageDropped(timestamp); + } } + mImageListenerMap.clear(); } - mImageListenerMap.clear(); } }