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

1 files changed, 69 insertions, 59 deletions

diff --git a/core/java/android/hardware/camera2/impl/CameraExtensionSessionImpl.java b/core/java/android/hardware/camera2/impl/CameraExtensionSessionImpl.java
index 8e7c7e0cfca8..0bb5046e21df 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;
-            }
+            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;
+                }
 
-            Long timestamp = img.getTimestamp();
-            if (mImageListenerMap.containsKey(timestamp)) {
-                Pair<Image, OnImageAvailableListener> entry = mImageListenerMap.remove(timestamp);
-                if (entry.second != null) {
-                    entry.second.onImageAvailable(reader, img);
+                Long timestamp = img.getTimestamp();
+                if (mImageListenerMap.containsKey(timestamp)) {
+                    Pair<Image, OnImageAvailableListener> 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 {
-                    Log.w(TAG, "Invalid image listener, dropping frame!");
-                    img.close();
+                    mImageListenerMap.put(timestamp, new Pair<>(img, null));
                 }
-            } else {
-                mImageListenerMap.put(img.getTimestamp(), new Pair<>(img, null));
-            }
 
-            notifyDroppedImages(timestamp);
+                notifyDroppedImages(timestamp);
+            }
         }
 
         private void notifyDroppedImages(long timestamp) {
-            Set<Long> timestamps = mImageListenerMap.keySet();
-            ArrayList<Long> removedTs = new ArrayList<>();
-            for (long ts : timestamps) {
-                if (ts < timestamp) {
-                    Log.e(TAG, "Dropped image with ts: " + ts);
-                    Pair<Image, OnImageAvailableListener> entry = mImageListenerMap.get(ts);
-                    if (entry.second != null) {
-                        entry.second.onImageDropped(ts);
-                    }
-                    if (entry.first != null) {
-                        entry.first.close();
+            synchronized (mInterfaceLock) {
+                Set<Long> timestamps = mImageListenerMap.keySet();
+                ArrayList<Long> removedTs = new ArrayList<>();
+                for (long ts : timestamps) {
+                    if (ts < timestamp) {
+                        Log.e(TAG, "Dropped image with ts: " + ts);
+                        Pair<Image, OnImageAvailableListener> entry = mImageListenerMap.get(ts);
+                        if (entry.second != null) {
+                            entry.second.onImageDropped(ts);
+                        }
+                        if (entry.first != null) {
+                            entry.first.close();
+                        }
+                        removedTs.add(ts);
                     }
-                    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<Image, OnImageAvailableListener> 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<Image, OnImageAvailableListener> 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<Image, OnImageAvailableListener> entry : mImageListenerMap.values()) {
-                if (entry.first != null) {
-                    entry.first.close();
+            synchronized (mInterfaceLock) {
+                for (Pair<Image, OnImageAvailableListener> entry : mImageListenerMap.values()) {
+                    if (entry.first != null) {
+                        entry.first.close();
+                    }
                 }
-            }
-            for (long timestamp : mImageListenerMap.keySet()) {
-                Pair<Image, OnImageAvailableListener> entry = mImageListenerMap.get(timestamp);
-                if (entry.second != null) {
-                    entry.second.onImageDropped(timestamp);
+                for (long timestamp : mImageListenerMap.keySet()) {
+                    Pair<Image, OnImageAvailableListener> entry = mImageListenerMap.get(timestamp);
+                    if (entry.second != null) {
+                        entry.second.onImageDropped(timestamp);
+                    }
                 }
+                mImageListenerMap.clear();
             }
-            mImageListenerMap.clear();
         }
     }