Skip to content

Commit

Permalink
Fix ImageReader may leak images when onDraw() not called (flutter#24272)
Browse files Browse the repository at this point in the history
  • Loading branch information
@eggfly
eggfly authored Feb 18, 2021
1 parent 625f453 commit e9e738c
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 136 deletions.
75 changes: 27 additions & 48 deletions shell/platform/android/io/flutter/embedding/android/FlutterImageView.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
import io.flutter.embedding.engine.renderer.FlutterRenderer;
import io.flutter.embedding.engine.renderer.RenderSurface;
import java.nio.ByteBuffer;
import java.util.LinkedList;
import java.util.Queue;

/**
* Paints a Flutter UI provided by an {@link android.media.ImageReader} onto a {@link
Expand All @@ -41,7 +39,6 @@
@TargetApi(19)
public class FlutterImageView extends View implements RenderSurface {
@NonNull private ImageReader imageReader;
@Nullable private Queue<Image> imageQueue;
@Nullable private Image currentImage;
@Nullable private Bitmap currentBitmap;
@Nullable private FlutterRenderer flutterRenderer;
Expand All @@ -57,13 +54,6 @@ public enum SurfaceKind {
/** The kind of surface. */
private SurfaceKind kind;

/**
* The number of images acquired from the current {@link android.media.ImageReader} that are
* waiting to be painted. This counter is decreased after calling {@link
* android.media.Image#close()}.
*/
private int pendingImages = 0;

/** Whether the view is attached to the Flutter render. */
private boolean isAttachedToFlutterRenderer = false;

Expand All @@ -89,7 +79,6 @@ public FlutterImageView(@NonNull Context context, @NonNull AttributeSet attrs) {
super(context, null);
this.imageReader = imageReader;
this.kind = kind;
this.imageQueue = new LinkedList<>();
init();
}

Expand Down Expand Up @@ -155,22 +144,14 @@ public void detachFromRenderer() {
return;
}
setAlpha(0.0f);
// Drop the lastest image as it shouldn't render this image if this view is
// Drop the latest image as it shouldn't render this image if this view is
// attached to the renderer again.
acquireLatestImage();
// Clear drawings.
currentBitmap = null;

// Close the images in the queue and clear the queue.
for (final Image image : imageQueue) {
image.close();
}
imageQueue.clear();
// Close and clear the current image if any.
if (currentImage != null) {
currentImage.close();
currentImage = null;
}
closeCurrentImage();
invalidate();
isAttachedToFlutterRenderer = false;
}
Expand All @@ -188,26 +169,20 @@ public boolean acquireLatestImage() {
if (!isAttachedToFlutterRenderer) {
return false;
}
// There's no guarantee that the image will be closed before the next call to
// `acquireLatestImage()`. For example, the device may not produce new frames if
// it's in sleep mode, so the calls to `invalidate()` will be queued up
// 1. `acquireLatestImage()` may return null if no new image is available.
// 2. There's no guarantee that `onDraw()` is called after `invalidate()`.
// For example, the device may not produce new frames if it's in sleep mode
// or some special Android devices so the calls to `invalidate()` queued up
// until the device produces a new frame.
//
// While the engine will also stop producing frames, there is a race condition.
//
// To avoid exceptions, check if a new image can be acquired.
int imageOpenedCount = imageQueue.size();
if (currentImage != null) {
imageOpenedCount++;
// 3. While the engine will also stop producing frames, there is a race condition.
final Image newImage = imageReader.acquireLatestImage();
if (newImage != null) {
// Only close current image after acquiring valid new image
closeCurrentImage();
currentImage = newImage;
invalidate();
}
if (imageOpenedCount < imageReader.getMaxImages()) {
final Image image = imageReader.acquireLatestImage();
if (image != null) {
imageQueue.add(image);
}
}
invalidate();
return !imageQueue.isEmpty();
return newImage != null;
}

/** Creates a new image reader with the provided size. */
Expand All @@ -218,32 +193,36 @@ public void resizeIfNeeded(int width, int height) {
if (width == imageReader.getWidth() && height == imageReader.getHeight()) {
return;
}
imageQueue.clear();
currentImage = null;

// Close resources.
closeCurrentImage();

// Close all the resources associated with the image reader,
// including the images.
imageReader.close();
// Image readers cannot be resized once created.
imageReader = createImageReader(width, height);
pendingImages = 0;
}

@Override
protected void onDraw(Canvas canvas) {
super.onDraw(canvas);

if (!imageQueue.isEmpty()) {
if (currentImage != null) {
currentImage.close();
}
currentImage = imageQueue.poll();
if (currentImage != null) {
updateCurrentBitmap();
}
if (currentBitmap != null) {
canvas.drawBitmap(currentBitmap, 0, 0, null);
}
}

private void closeCurrentImage() {
// Close and clear the current image if any.
if (currentImage != null) {
currentImage.close();
currentImage = null;
}
}

@TargetApi(29)
private void updateCurrentBitmap() {
if (android.os.Build.VERSION.SDK_INT >= 29) {
Expand Down
111 changes: 23 additions & 88 deletions shell/platform/android/test/io/flutter/embedding/android/FlutterViewTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -658,36 +658,19 @@ public void flutterImageView_acquiresImageAndInvalidates() {
verify(imageView, times(1)).invalidate();
}

@Test
public void flutterImageView_acquireLatestImageReturnsFalse() {
final ImageReader mockReader = mock(ImageReader.class);
when(mockReader.getMaxImages()).thenReturn(2);

final FlutterImageView imageView =
spy(
new FlutterImageView(
RuntimeEnvironment.application,
mockReader,
FlutterImageView.SurfaceKind.background));

assertFalse(imageView.acquireLatestImage());

final FlutterJNI jni = mock(FlutterJNI.class);
imageView.attachToRenderer(new FlutterRenderer(jni));

when(mockReader.acquireLatestImage()).thenReturn(null);
assertFalse(imageView.acquireLatestImage());
}

@Test
@SuppressLint("WrongCall") /*View#onDraw*/
public void flutterImageView_acquiresMaxImagesAtMost() {
public void flutterImageView_acquiresImageClosesPreviousImageUnlessNoNewImage() {
final ImageReader mockReader = mock(ImageReader.class);
when(mockReader.getMaxImages()).thenReturn(3);

final Image mockImage = mock(Image.class);
when(mockImage.getPlanes()).thenReturn(new Plane[0]);
when(mockReader.acquireLatestImage()).thenReturn(mockImage);
// Mock no latest image on the second time
when(mockReader.acquireLatestImage())
.thenReturn(mockImage)
.thenReturn(null)
.thenReturn(mockImage);

final FlutterImageView imageView =
spy(
Expand All @@ -700,33 +683,27 @@ public void flutterImageView_acquiresMaxImagesAtMost() {
imageView.attachToRenderer(new FlutterRenderer(jni));
doNothing().when(imageView).invalidate();

assertTrue(imageView.acquireLatestImage()); // 1 image
assertTrue(imageView.acquireLatestImage()); // 2 images
assertTrue(imageView.acquireLatestImage()); // 3 images
assertTrue(imageView.acquireLatestImage()); // 3 images
verify(mockReader, times(3)).acquireLatestImage();
assertTrue(imageView.acquireLatestImage()); // No previous, acquire latest image
assertFalse(
imageView.acquireLatestImage()); // Mock no image when acquire, don't close, and assertFalse
assertTrue(imageView.acquireLatestImage()); // Acquire latest image and close previous
assertTrue(imageView.acquireLatestImage()); // Acquire latest image and close previous
assertTrue(imageView.acquireLatestImage()); // Acquire latest image and close previous
verify(mockImage, times(3)).close(); // Close 3 times

imageView.onDraw(mock(Canvas.class)); // 3 images
assertTrue(imageView.acquireLatestImage()); // 3 images
verify(mockReader, times(3)).acquireLatestImage();
imageView.onDraw(mock(Canvas.class)); // Draw latest image

imageView.onDraw(mock(Canvas.class)); // 2 images
assertTrue(imageView.acquireLatestImage()); // 3 images
verify(mockReader, times(4)).acquireLatestImage();
assertTrue(imageView.acquireLatestImage()); // acquire latest image and close previous

imageView.onDraw(mock(Canvas.class)); // 2 images
imageView.onDraw(mock(Canvas.class)); // 1 image
imageView.onDraw(mock(Canvas.class)); // 1 image
imageView.onDraw(mock(Canvas.class)); // Draw latest image
imageView.onDraw(mock(Canvas.class)); // Draw latest image
imageView.onDraw(mock(Canvas.class)); // Draw latest image

assertTrue(imageView.acquireLatestImage()); // 2 images
assertTrue(imageView.acquireLatestImage()); // 3 images
assertTrue(imageView.acquireLatestImage()); // 3 images
assertTrue(imageView.acquireLatestImage()); // 3 images
verify(mockReader, times(6)).acquireLatestImage();
}

@Test
public void flutterImageView_detachFromRendererClosesAllImages() {
public void flutterImageView_detachFromRendererClosesPreviousImage() {
final ImageReader mockReader = mock(ImageReader.class);
when(mockReader.getMaxImages()).thenReturn(2);

Expand All @@ -746,54 +723,12 @@ public void flutterImageView_detachFromRendererClosesAllImages() {
doNothing().when(imageView).invalidate();
imageView.acquireLatestImage();
imageView.acquireLatestImage();
imageView.detachFromRenderer();

verify(mockImage, times(2)).close();
}

@Test
@SuppressLint("WrongCall") /*View#onDraw*/
public void flutterImageView_onDrawClosesAllImages() {
final ImageReader mockReader = mock(ImageReader.class);
when(mockReader.getMaxImages()).thenReturn(2);

final Image mockImage = mock(Image.class);
when(mockImage.getPlanes()).thenReturn(new Plane[0]);
when(mockReader.acquireLatestImage()).thenReturn(mockImage);

final FlutterImageView imageView =
spy(
new FlutterImageView(
RuntimeEnvironment.application,
mockReader,
FlutterImageView.SurfaceKind.background));

final FlutterJNI jni = mock(FlutterJNI.class);
imageView.attachToRenderer(new FlutterRenderer(jni));

doNothing().when(imageView).invalidate();
imageView.acquireLatestImage();
imageView.acquireLatestImage();

imageView.onDraw(mock(Canvas.class));
imageView.onDraw(mock(Canvas.class));

// 1 image is closed and 1 is active.
verify(mockImage, times(1)).close();
verify(mockReader, times(2)).acquireLatestImage();

// This call doesn't do anything because there isn't
// an image in the queue.
imageView.onDraw(mock(Canvas.class));
verify(mockImage, times(1)).close();

// Aquire another image and push it to the queue.
imageView.acquireLatestImage();
verify(mockReader, times(3)).acquireLatestImage();

// Then, the second image is closed.
imageView.onDraw(mock(Canvas.class));
verify(mockImage, times(2)).close();
imageView.detachFromRenderer();
// There's an acquireLatestImage() in detachFromRenderer(),
// so it will be 2 times called close() inside detachFromRenderer()
verify(mockImage, times(3)).close();
}

@Test
Expand Down

0 comments on commit e9e738c

Please sign in to comment.