Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

ImageWriter: remove mCloseLock guard from queue and dequeue operations

Browse Source 
mCloseLock currently guards #queueInputImage, #dequeueInputImage, and
#postEventFromNative. However, there have been cases reported where a
contention between #postEventFromNative and #queueInputImage would lead
to a deadlock. This CL removes the mCloseLock guard from #queueInputImage
and #dequeInputImage as the native implementations are already
synchronized on `this` and there are no reported bugs from race
conditions in those two functions.

It also drops the mCloseLock guard from #postEventFromNative and moves
the responsibility of ensuring ImageWriter is valid to the
ListenerHandler. This ensures that mCloseLock is never in contention
in the JNI thread that calls #postEventFromNative.

#close() is still protected by mCloseLock to prevent multiple
simultaneous closes.

Bug: 263395157
Test: Existing ImageWriter tests pass
      Ran the sequence that led to deadlock 30 times without a single
      deadlock.
Change-Id: I5b0d367cce7df9401386e5a53c5e9b539d8c1f40
This commit is contained in:
Avichal Rakesh
2022-12-29 03:12:52 -08:00
parent da209b100a
commit eb039c3f01
1 changed files with 52 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files

110
media/java/android/media/ImageWriter.java
View File

@@ -452,17 +452,15 @@ public class ImageWriter implements AutoCloseable {
* @see Image#close
*/
public Image dequeueInputImage() {
synchronized (mCloseLock) {
if (mDequeuedImages.size() >= mMaxImages) {
throw new IllegalStateException(
"Already dequeued max number of Images " + mMaxImages);
}
WriterSurfaceImage newImage = new WriterSurfaceImage(this);
nativeDequeueInputImage(mNativeContext, newImage);
mDequeuedImages.add(newImage);
newImage.mIsImageValid = true;
return newImage;
if (mDequeuedImages.size() >= mMaxImages) {
throw new IllegalStateException(
"Already dequeued max number of Images " + mMaxImages);
}
WriterSurfaceImage newImage = new WriterSurfaceImage(this);
nativeDequeueInputImage(mNativeContext, newImage);
mDequeuedImages.add(newImage);
newImage.mIsImageValid = true;
return newImage;
}
/**
@@ -521,52 +519,50 @@ public class ImageWriter implements AutoCloseable {
throw new IllegalArgumentException("image shouldn't be null");
}
synchronized (mCloseLock) {
boolean ownedByMe = isImageOwnedByMe(image);
if (ownedByMe && !(((WriterSurfaceImage) image).mIsImageValid)) {
throw new IllegalStateException("Image from ImageWriter is invalid");
boolean ownedByMe = isImageOwnedByMe(image);
if (ownedByMe && !(((WriterSurfaceImage) image).mIsImageValid)) {
throw new IllegalStateException("Image from ImageWriter is invalid");
}
// For images from other components that have non-null owner, need to detach first,
// then attach. Images without owners must already be attachable.
if (!ownedByMe) {
if ((image.getOwner() instanceof ImageReader)) {
ImageReader prevOwner = (ImageReader) image.getOwner();
prevOwner.detachImage(image);
} else if (image.getOwner() != null) {
throw new IllegalArgumentException(
"Only images from ImageReader can be queued to"
+ " ImageWriter, other image source is not supported yet!");
}
// For images from other components that have non-null owner, need to detach first,
// then attach. Images without owners must already be attachable.
if (!ownedByMe) {
if ((image.getOwner() instanceof ImageReader)) {
ImageReader prevOwner = (ImageReader) image.getOwner();
attachAndQueueInputImage(image);
// This clears the native reference held by the original owner.
// When this Image is detached later by this ImageWriter, the
// native memory won't be leaked.
image.close();
return;
}
prevOwner.detachImage(image);
} else if (image.getOwner() != null) {
throw new IllegalArgumentException(
"Only images from ImageReader can be queued to"
+ " ImageWriter, other image source is not supported yet!");
}
Rect crop = image.getCropRect();
nativeQueueInputImage(mNativeContext, image, image.getTimestamp(), image.getDataSpace(),
crop.left, crop.top, crop.right, crop.bottom, image.getTransform(),
image.getScalingMode());
attachAndQueueInputImage(image);
// This clears the native reference held by the original owner.
// When this Image is detached later by this ImageWriter, the
// native memory won't be leaked.
image.close();
return;
}
Rect crop = image.getCropRect();
nativeQueueInputImage(mNativeContext, image, image.getTimestamp(), image.getDataSpace(),
crop.left, crop.top, crop.right, crop.bottom, image.getTransform(),
image.getScalingMode());
/**
* Only remove and cleanup the Images that are owned by this
* ImageWriter. Images detached from other owners are only temporarily
* owned by this ImageWriter and will be detached immediately after they
* are released by downstream consumers, so there is no need to keep
* track of them in mDequeuedImages.
*/
if (ownedByMe) {
mDequeuedImages.remove(image);
// Do not call close here, as close is essentially cancel image.
WriterSurfaceImage wi = (WriterSurfaceImage) image;
wi.clearSurfacePlanes();
wi.mIsImageValid = false;
}
/**
* Only remove and cleanup the Images that are owned by this
* ImageWriter. Images detached from other owners are only temporarily
* owned by this ImageWriter and will be detached immediately after they
* are released by downstream consumers, so there is no need to keep
* track of them in mDequeuedImages.
*/
if (ownedByMe) {
mDequeuedImages.remove(image);
// Do not call close here, as close is essentially cancel image.
WriterSurfaceImage wi = (WriterSurfaceImage) image;
wi.clearSurfacePlanes();
wi.mIsImageValid = false;
}
}
@@ -702,11 +698,11 @@ public class ImageWriter implements AutoCloseable {
*/
@Override
public void close() {
setOnImageReleasedListener(null, null);
synchronized (mCloseLock) {
if (!mIsWriterValid) {
return;
}
setOnImageReleasedListener(null, null);
for (Image image : mDequeuedImages) {
image.close();
}
@@ -836,14 +832,12 @@ public class ImageWriter implements AutoCloseable {
}
final Handler handler;
final boolean isWriterValid;
synchronized (iw.mListenerLock) {
handler = iw.mListenerHandler;
}
synchronized (iw.mCloseLock) {
isWriterValid = iw.mIsWriterValid;
}
if (handler != null && isWriterValid) {
if (handler != null) {
// The ListenerHandler will take care of ensuring that the parent ImageWriter is valid
handler.sendEmptyMessage(0);
}
}