Fix ImageReader onImageAvailable synchronization
This avoids a race where close() can return while there are still
onImageAvailable callbacks pending.
Bug: 10666923
Change-Id: Ic519b68f3132ceb7f95a9a42ebd1032c1638fbf5
diff --git a/media/java/android/media/ImageReader.java b/media/java/android/media/ImageReader.java
index aee8362..f9e48d4 100644
--- a/media/java/android/media/ImageReader.java
+++ b/media/java/android/media/ImageReader.java
@@ -20,6 +20,7 @@
import android.graphics.PixelFormat;
import android.os.Handler;
import android.os.Looper;
+import android.os.Message;
import android.view.Surface;
import java.lang.ref.WeakReference;
@@ -377,17 +378,21 @@
* @throws IllegalArgumentException
* If no handler specified and the calling thread has no looper.
*/
- public void setOnImageAvailableListener(OnImageAvailableListener listener, Handler handler) {
- mImageListener = listener;
-
- Looper looper;
- mHandler = handler;
- if (listener != null && mHandler == null) {
- if ((looper = Looper.myLooper()) != null) {
- mHandler = new Handler();
+ public void setOnImageAvailableListener(OnImageAvailableListener listener, Handler handler) {
+ synchronized (mListenerLock) {
+ if (listener != null) {
+ Looper looper = handler != null ? handler.getLooper() : Looper.myLooper();
+ if (looper == null) {
+ throw new IllegalArgumentException(
+ "handler is null but the current thread is not a looper");
+ }
+ if (mListenerHandler == null || mListenerHandler.getLooper() != looper) {
+ mListenerHandler = new ListenerHandler(looper);
+ }
+ mListener = listener;
} else {
- throw new IllegalArgumentException(
- "Looper doesn't exist in the calling thread");
+ mListener = null;
+ mListenerHandler = null;
}
}
}
@@ -426,6 +431,7 @@
*/
@Override
public void close() {
+ setOnImageAvailableListener(null, null);
nativeClose();
}
@@ -474,6 +480,9 @@
/**
* Called from Native code when an Event happens.
+ *
+ * This may be called from an arbitrary Binder thread, so access to the ImageReader must be
+ * synchronized appropriately.
*/
private static void postEventFromNative(Object selfRef) {
@SuppressWarnings("unchecked")
@@ -483,16 +492,16 @@
return;
}
- if (ir.mHandler != null && ir.mImageListener != null) {
- ir.mHandler.post(new Runnable() {
- @Override
- public void run() {
- ir.mImageListener.onImageAvailable(ir);
- }
- });
+ final Handler handler;
+ synchronized (ir.mListenerLock) {
+ handler = ir.mListenerHandler;
+ }
+ if (handler != null) {
+ handler.sendEmptyMessage(0);
}
}
+
private final int mWidth;
private final int mHeight;
private final int mFormat;
@@ -500,14 +509,35 @@
private final int mNumPlanes;
private final Surface mSurface;
- private Handler mHandler;
- private OnImageAvailableListener mImageListener;
+ private final Object mListenerLock = new Object();
+ private OnImageAvailableListener mListener;
+ private ListenerHandler mListenerHandler;
/**
* This field is used by native code, do not access or modify.
*/
private long mNativeContext;
+ /**
+ * This custom handler runs asynchronously so callbacks don't get queued behind UI messages.
+ */
+ private final class ListenerHandler extends Handler {
+ public ListenerHandler(Looper looper) {
+ super(looper, null, true /*async*/);
+ }
+
+ @Override
+ public void handleMessage(Message msg) {
+ OnImageAvailableListener listener;
+ synchronized (mListenerLock) {
+ listener = mListener;
+ }
+ if (listener != null) {
+ listener.onImageAvailable(ImageReader.this);
+ }
+ }
+ }
+
private class SurfaceImage extends android.media.Image {
public SurfaceImage() {
mIsImageValid = false;