Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix acquireLatestImage exceeds maxImage and cause IllegalStateException. #20475

Closed
wants to merge 1 commit into from

Conversation

plateaukao
Copy link

@plateaukao plateaukao commented Aug 13, 2020

Description

When using PlatformViewLink widget to wrap a native WebView in Android, in certain scenarios, the app will be crashed with following callstack.

Scenario

  • launch a screen with PlatformViewLink` wrapped WebView, while it's loading the content, press HOME key on android device.
  • It may crash the flutter app with around 30% to 60% percent crash rate.

W/ImageReader_JNI(18777): Unable to acquire a buffer item, very likely client tried to acquire more than maxImages buffers E/flutter (18777): [ERROR:flutter/shell/platform/android/platform_view_android_jni_impl.cc(43)] java.lang.IllegalStateException: maxImages (3) has already been acquired, call #close before acquiring more. E/flutter (18777): at android.media.ImageReader.acquireNextImage(ImageReader.java:513) E/flutter (18777): at android.media.ImageReader.acquireLatestImage(ImageReader.java:397) E/flutter (18777): at io.flutter.embedding.android.FlutterImageView.acquireLatestImage(FlutterImageView.java:186) E/flutter (18777): at io.flutter.embedding.android.FlutterView.acquireLatestImageViewFrame(FlutterView.java:1039) E/flutter (18777): at io.flutter.plugin.platform.PlatformViewsController.onEndFrame(PlatformViewsController.java:780) E/flutter (18777): at io.flutter.embedding.engine.FlutterJNI.onEndFrame(FlutterJNI.java:856) E/flutter (18777): at android.os.MessageQueue.nativePollOnce(Native Method) E/flutter (18777): at android.os.MessageQueue.next(MessageQueue.java:336) E/flutter (18777): at android.os.Looper.loop(Looper.java:197) E/flutter (18777): at android.app.ActivityThread.main(ActivityThread.java:8016) E/flutter (18777): at java.lang.reflect.Method.invoke(Native Method) E/flutter (18777): at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493) E/flutter (18777): at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1076) E/flutter (18777): F/flutter (18777): [FATAL:flutter/shell/platform/android/platform_view_android_jni_impl.cc(1241)] Check failed: CheckException(env). F/libc (18777): Fatal signal 6 (SIGABRT), code -6 (SI_TKILL) in tid 18777 (twshopping.beta), pid 18777 (twshopping.beta)

Here's the official document for acquireLatestImage() https://developer.android.com/reference/android/media/ImageReader#acquireLatestImage()

It says that

This operation will fail by throwing an IllegalStateException if maxImages have been acquired with acquireNextImage() or acquireLatestImage().

So, I tried to catch IllegalStateException to prevent the native crash.
As for in what circumstances it will cause this scenario, since I am not so familiar with flutter/engine. It may need other's help to figure it out.

Related Issues

Maybe related to the issue mentioned in
https://github.com/flutter/engine/pull/19325/files#
https://github.com/flutter/engine/pull/19487/files

Tests

I added the following tests:

Replace this with a list of the tests that you added as part of this PR. A
change in behaviour with no test covering it will likely get reverted
accidentally sooner or later. PRs must include tests for all
changed/updated/fixed behaviors. See testing the engine for instructions on
writing and running engine tests.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the contributor guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the C++, Objective-C, Java style guides for the engine.
  • I read the tree hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation.
  • All existing and new tests are passing.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

1 similar comment
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@auto-assign auto-assign bot requested a review from gw280 August 13, 2020 11:22
@plateaukao
Copy link
Author

@googlebot I signed it!

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

1 similar comment
@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@aaron-chu
Copy link

aaron-chu commented Aug 16, 2020

I filed an issue flutter/flutter#63897 about this and provided the reproduced steps to reproduce it by 100%. Hope this can help to offer more information to solve this problem.

@blasten
Copy link

blasten commented Aug 28, 2020

Thanks for the PR. I'd prefer to fix the root cause of this issue. #20842 attempts to do that. Would you be able to verify the fix?

@plateaukao plateaukao closed this Sep 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants