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

Crash on Android when tapping on camera icon in compose box, in release mode #879

Closed
rajveermalviya opened this issue Aug 11, 2024 · 9 comments · Fixed by #887
Closed

Crash on Android when tapping on camera icon in compose box, in release mode #879

rajveermalviya opened this issue Aug 11, 2024 · 9 comments · Fixed by #887
Labels
a-compose Compose box, autocomplete, attaching files/images

Comments

@rajveermalviya
Copy link
Collaborator

Logs:

$ flutter run --release
...
...
...
I/flutter (14099): [IMPORTANT:flutter/shell/platform/android/android_context_vk_impeller.cc(59)] Using the Impeller rendering backend (Vulkan).
I/flutter (14099): [IMPORTANT:flutter/shell/platform/android/android_context_vk_impeller.cc(59)] Using the Impeller rendering backend (Vulkan).
E/AndroidRuntime(14099): FATAL EXCEPTION: flutter-worker-1
E/AndroidRuntime(14099): Process: com.zulip.flutter, PID: 14099
E/AndroidRuntime(14099): java.lang.IncompatibleClassChangeError: Class 'android.content.res.XmlBlock$Parser' does not implement interface 'w7.a' in call to 'int w7.a.next()' (declaration of 'androidx.core.content.b' appears in /data/app/~~SJEWO15N5DtF7C3Opydr8A==/com.zulip.flutter-7Osxw0qoFXmxClFDo-wbPg==/base.apk)
E/AndroidRuntime(14099):        at androidx.core.content.b.j(Unknown Source:19)
E/AndroidRuntime(14099):        at androidx.core.content.b.g(Unknown Source:11)
E/AndroidRuntime(14099):        at androidx.core.content.b.h(Unknown Source:1)
E/AndroidRuntime(14099):        at p6.l$b.b(Unknown Source:2)
E/AndroidRuntime(14099):        at p6.l.P(Unknown Source:51)
E/AndroidRuntime(14099):        at p6.l.X(Unknown Source:35)
E/AndroidRuntime(14099):        at p6.n.j(Unknown Source:68)
E/AndroidRuntime(14099):        at p6.s$f.n(Unknown Source:33)
E/AndroidRuntime(14099):        at p6.s$f.f(Unknown Source:0)
E/AndroidRuntime(14099):        at p6.t.a(Unknown Source:2)
E/AndroidRuntime(14099):        at j6.a$b.a(Unknown Source:17)
E/AndroidRuntime(14099):        at a6.c.l(Unknown Source:18)
E/AndroidRuntime(14099):        at a6.c.m(Unknown Source:41)
E/AndroidRuntime(14099):        at a6.c.i(Unknown Source:0)
E/AndroidRuntime(14099):        at a6.b.run(Unknown Source:12)
E/AndroidRuntime(14099):        at a6.c$h.d(Unknown Source:20)
E/AndroidRuntime(14099):        at a6.c$h.e(Unknown Source:0)
E/AndroidRuntime(14099):        at a6.c$h.b(Unknown Source:0)
E/AndroidRuntime(14099):        at a6.d.run(Unknown Source:2)
E/AndroidRuntime(14099):        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
E/AndroidRuntime(14099):        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
E/AndroidRuntime(14099):        at java.lang.Thread.run(Thread.java:923)

Application finished.
@rajveermalviya
Copy link
Collaborator Author

Interestingly, the crash doesn't occur when ran with flutter run --release -Pshrink=false.

@rajveermalviya
Copy link
Collaborator Author

Initially occured on my personal device, but just tried reproducing it in Emulator (API 34) and it reproduces there too; crash doesn't occur when built with -Pshrink=false on the emulator too.

@gnprice
Copy link
Member

gnprice commented Aug 11, 2024

Huh interesting, thanks for the report.

This definitely used to work. Can you find a previous version where it did work, and then bisect to find what change made it stop working?

(I'd guess the relevant change is either in Flutter upstream, or in package:image_picker — we don't really have code of our own that I'd expect to interact with whatever triggers this bug.)

@gnprice gnprice added this to the Beta 3: Summer 2024 milestone Aug 11, 2024
@gnprice gnprice added the a-compose Compose box, autocomplete, attaching files/images label Aug 11, 2024
@rajveermalviya
Copy link
Collaborator Author

rajveermalviya commented Aug 12, 2024

Okay, so git bisect with flutter checkout 3.22.0-29.0.pre between v0.0.14 (good) and v0.0.15 (bad), it leads to 6e8d8b0.

Which seems unrelated, but could be something to do with video_player's Android build config.

Found an upstream report with similar error message — flutter/flutter#146266

@rajveermalviya
Copy link
Collaborator Author

Just tested that the crash doesn't occur with updated AGP and other related dependecies, including enabling coreLibraryDesugaringEnabled to fix an error encoutered while updating AGP for package:flutter_local_notifications, see — https://pub.dev/packages/flutter_local_notifications#gradle-setup.

diff --git a/android/app/build.gradle b/android/app/build.gradle
index dd4178c..4f01760 100644
--- a/android/app/build.gradle
+++ b/android/app/build.gradle
@@ -23,9 +23,10 @@ android {
     namespace "com.zulip.flutter"
 
     compileSdkVersion flutter.compileSdkVersion
-    ndkVersion flutter.ndkVersion
+    ndkVersion '27.0.12077973'
 
     compileOptions {
+        coreLibraryDesugaringEnabled true
         sourceCompatibility JavaVersion.VERSION_1_8
         targetCompatibility JavaVersion.VERSION_1_8
     }
@@ -97,4 +98,5 @@ flutter {
 
 dependencies {
     implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlinVersion"
+    coreLibraryDesugaring 'com.android.tools:desugar_jdk_libs:2.0.4'
 }
diff --git a/android/gradle.properties b/android/gradle.properties
index 740bb74..ee5e36e 100644
--- a/android/gradle.properties
+++ b/android/gradle.properties
@@ -6,11 +6,11 @@ android.enableJetifier=true
 # Defining them here makes them available both in
 # settings.gradle and in the build.gradle files.
 
-agpVersion=8.1.4
+agpVersion=8.5.0
 
 # Generally update this to the version found in recent releases
 # of Android Studio, as listed in this table:
 #   https://kotlinlang.org/docs/releases.html#release-details
 # A helpful discussion is at:
 #   https://stackoverflow.com/a/74425347
-kotlinVersion=1.9.10
+kotlinVersion=2.0.10
diff --git a/android/gradle/wrapper/gradle-wrapper.properties b/android/gradle/wrapper/gradle-wrapper.properties
index 07e42d5..545935f 100644
--- a/android/gradle/wrapper/gradle-wrapper.properties
+++ b/android/gradle/wrapper/gradle-wrapper.properties
@@ -6,7 +6,7 @@
 #  the wrapper is the one from the new Gradle too.)
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-8.6-all.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-8.9-all.zip
 networkTimeout=10000
 validateDistributionUrl=true
 zipStoreBase=GRADLE_USER_HOME

Alternative (and simpler) fix is to disable R8 fullmode temporarily, but it might affect build sizes:

# android/gradle.properties
android.enableR8.fullMode=false

Though the root cause still remains unknown, the different AGP release notes doesn't say much — https://developer.android.com/build/releases/past-releases/agp-8-4-0-release-notes.

@gnprice
Copy link
Member

gnprice commented Aug 12, 2024

Fascinating, thanks. Yeah, presumably the trigger must be something in the package:video_player Android build config.

Also interesting that those upgrades fix it. We should probably take them anyway... except that NDK upgrade, where it'd be nice to stick to the one from Flutter upstream. Do things work if you leave that one out?

@rajveermalviya
Copy link
Collaborator Author

Keeping the ndk version to flutter.ndkVersion results in following warnings (in red text) but the build does succeed:

Your project is configured with Android NDK 23.1.7779620, but the following plugin(s) depend on a different Android NDK version:
- app_settings requires Android NDK 26.1.10909125
- device_info_plus requires Android NDK 26.1.10909125
- file_picker requires Android NDK 26.1.10909125
- firebase_core requires Android NDK 26.1.10909125
- firebase_messaging requires Android NDK 26.1.10909125
- flutter_local_notifications requires Android NDK 26.1.10909125
- flutter_plugin_android_lifecycle requires Android NDK 26.1.10909125
- image_picker_android requires Android NDK 26.1.10909125
- integration_test requires Android NDK 26.1.10909125
- package_info_plus requires Android NDK 26.1.10909125
- path_provider_android requires Android NDK 26.1.10909125
- share_plus requires Android NDK 26.1.10909125
- sqlite3_flutter_libs requires Android NDK 26.1.10909125
- url_launcher_android requires Android NDK 26.1.10909125
- video_player_android requires Android NDK 26.1.10909125
- zulip_plugin requires Android NDK 26.1.10909125
Fix this issue by using the highest Android NDK version (they are backward compatible).
Add the following to /Users/rajveer/Projects/zulip-flutter/android/app/build.gradle:

    android {
        ndkVersion = "26.1.10909125"
        ...
    }

To avoid this, the above diff updates it to the latest LTS version27.0.12077973


Also, interestingly removing the explicit ndkVersion line from build.gradle results in a successful build without any warnings.

@gnprice
Copy link
Member

gnprice commented Aug 12, 2024

Interesting. I wonder what the point is of flutter.ndkVersion if plugins (including first-party plugins, which many on that list are) can end up requiring a newer version, and if things work fine when the app doesn't specify it.

I guess let's just delete that line, then, and let the NDK version be determined automatically from the other things in the build.

And then a PR that does that and the other Android-related upgrades in your diff above would be great. For the branch, please split the different changes into different commits: Gradle, AGP, Kotlin, NDK, coreLibraryDesugaring (in some order). That way they can each have their own commit message pointing to relevant release notes.

For example commit messages… well hmm, it looks like we've been neglecting these upgrades a bit (based on scanning git log --stat -p android/app/build.gradle android/gradle/wrapper/gradle-wrapper.properties). 🙂
(edit: ah no, it's that that git log command needed to include android/gradle.properties. That's where our AGP version and Kotlin version live. Discovered this at #887 (comment) .)
One good example is:
2599a99 deps android: Upgrade Gradle to 8.6, the latest

You can find more examples in the zulip-mobile repo, with git log --stat -p --grep version: --invert-grep android/app/build.gradle android/gradle/wrapper/gradle-wrapper.properties.

gnprice pushed a commit to rajveermalviya/zulip-flutter that referenced this issue Aug 13, 2024
Specifying the NDK version here doesn't seem to do us any good.

This value provided by the `flutter` tool is currently 23.1.7779620.
If we upgrade the Android Gradle plugin (AGP) to 8.5.0, its current
latest, then we start getting build warnings:
  zulip#879 (comment)
saying that a bunch of plugins -- including first-party plugins -- are
all using NDK 26.1.10909125 instead, and telling us to switch to that.

Conversely, if we leave this line out, then the same AGP upgrade
causes no warnings and works just fine.

So let the version float.  This does contradict AGP's own recommendation:
  https://developer.android.com/studio/projects/configure-agp-ndk
which is to use this `ndkVersion` property.  But if a bunch of our
plugins are going to use a floating version anyway, it doesn't seem
helpful to try to hang back.

[greg: added commit-message body]
rajveermalviya added a commit to rajveermalviya/zulip-flutter that referenced this issue Aug 13, 2024
Specifying the NDK version here doesn't seem to do us any good.

This value provided by the `flutter` tool is currently 23.1.7779620.
If we upgrade the Android Gradle plugin (AGP) to 8.5.0, its current
latest, then we start getting build warnings:
  zulip#879 (comment)
saying that a bunch of plugins -- including first-party plugins -- are
all using NDK 26.1.10909125 instead, and telling us to switch to that.

Conversely, if we leave this line out, then the same AGP upgrade
causes no warnings and works just fine.

So let the version float.  This does contradict AGP's own recommendation:
  https://developer.android.com/studio/projects/configure-agp-ndk
which is to use this `ndkVersion` property.  But if a bunch of our
plugins are going to use a floating version anyway, it doesn't seem
helpful to try to hang back.

[greg: added commit-message body]
rajveermalviya added a commit to rajveermalviya/zulip-flutter that referenced this issue Aug 13, 2024
Release notes:
  https://developer.android.com/build/releases/past-releases/agp-8-2-0-release-notes
  https://developer.android.com/build/releases/past-releases/agp-8-3-0-release-notes
  https://developer.android.com/build/releases/past-releases/agp-8-4-0-release-notes
  https://developer.android.com/build/releases/gradle-plugin (for 8.5, for now)

Some of the release notes (especially 8.2) are very thin; it's not
clear if there are other changes that aren't being described, or if
they just didn't change much.

Changes possibly affecting us:
 * 8.3 adds "precise resource shrinking", which seems to be an
   additional form of resource shrinking.
 * As usual, a new AGP version requires a new Android Studio version.
   AGP 8.5 requires Android Studio Koala.

[greg: added reading of release notes]

Fixes: zulip#879
@gnprice
Copy link
Member

gnprice commented Aug 14, 2024

Also updated that upstream issue (flutter/flutter#146266 (comment)) with our findings here. In particular it doesn't look like it was previously known on that issue thread that an AGP upgrade resolves it, so that should be helpful for other people hitting the issue.

rajveermalviya added a commit to rajveermalviya/zulip-flutter that referenced this issue Aug 14, 2024
Release notes:
  https://developer.android.com/build/releases/past-releases/agp-8-2-0-release-notes
  https://developer.android.com/build/releases/past-releases/agp-8-3-0-release-notes
  https://developer.android.com/build/releases/past-releases/agp-8-4-0-release-notes
  https://developer.android.com/build/releases/gradle-plugin (for 8.5, for now)

Some of the release notes (especially 8.2) are very thin; it's not
clear if there are other changes that aren't being described, or if
they just didn't change much.

Changes possibly affecting us:
 * 8.3 adds "precise resource shrinking", which seems to be an
   additional form of resource shrinking.
 * As usual, a new AGP version requires a new Android Studio version.
   AGP 8.5 requires Android Studio Koala.

[greg: added reading of release notes]

Fixes: zulip#879
gnprice pushed a commit to rajveermalviya/zulip-flutter that referenced this issue Aug 15, 2024
Specifying the NDK version here doesn't seem to do us any good.

This value provided by the `flutter` tool is currently 23.1.7779620.
If we upgrade the Android Gradle plugin (AGP) to 8.5.2, its current
latest, then we start getting build warnings:
  zulip#879 (comment)
saying that a bunch of plugins -- including first-party plugins -- are
all using NDK 26.1.10909125 instead, and telling us to switch to that.

Conversely, if we leave this line out, then the same AGP upgrade
causes no warnings and works just fine.

So let the version float.  This does contradict AGP's own recommendation:
  https://developer.android.com/studio/projects/configure-agp-ndk
which is to use this `ndkVersion` property.  But if a bunch of our
plugins are going to use a floating version anyway, it doesn't seem
helpful to try to hang back.

[greg: added commit-message body]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-compose Compose box, autocomplete, attaching files/images
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants