dart-lang / native

Dart packages related to FFI and native assets bundling.
BSD 3-Clause "New" or "Revised" License
83 stars 27 forks source link

NDK crash in `libdartjni.so (GetApplicationContext+60)` when uploading to Play Store #1150

Open lukehutch opened 2 weeks ago

lukehutch commented 2 weeks ago

I am getting an NDK crash in libdartjni.so (GetApplicationContext+60) when I upload my APK to the Play Store.

image

Any ideas?

HosseinYousefi commented 2 weeks ago
lukehutch commented 2 weeks ago
  • What version of package:jni are you using?

jni 0.7.3 -- required by cronet_http 1.2.0 (which is the latest version currently).

  • Will this happen If you override the dependency to use the latest version of package:jni?

Do I do that in my project via dependency_override, or do I need to fork cronet_http to override the dependency there?

Do you know if cronet_http works with jni 0.9.2? I'm worried about causing runtime crashes by messing with dependency versions of NDK projects...

  • Can you create a minimal reproducible repo that has this problem?

I would have to create a whole new app consisting of just the cronet client, do all the Play Store setup for the app, and upload it for testing, to try to recreate the crash with the smallest amount of code possible (and I really can't afford the time to do something like that for a few weeks, because I'm about to launch my app).

It doesn't crash on my local device or emulator, but I don't know what would be different about the Play Store...

lukehutch commented 2 weeks ago

Here is how I call into cronet, however... this would be close to my minimal repro case:

void main() async {
  // Set up better http clients on Android and iOS
  if (Platform.isAndroid) {
    final engine =
        CronetEngine.build(cacheMode: CacheMode.memory, cacheMaxSize: 1000000);
    runWithClient(_run, () => CronetClient.fromCronetEngine(engine));
  } else if (Platform.isIOS || Platform.isMacOS) {
    final config = URLSessionConfiguration.ephemeralSessionConfiguration()
      ..cache = URLCache.withCapacity(memoryCapacity: 1000000);
    runWithClient(_run, () => CupertinoClient.fromSessionConfiguration(config));
  } else {
    // Run with the default IOClient
    _run();
  }
}

void _run() async {
  // Must come first
  WidgetsFlutterBinding.ensureInitialized();

  // Call runApp from here...
}
HosseinYousefi commented 2 weeks ago

Thanks for the quick reply.

Do I do that in my project via dependency_override, or do I need to fork cronet_http to override the dependency there?

Do you know if cronet_http works with jni 0.9.2? I'm worried about causing runtime crashes by messing with dependency versions of NDK projects...

You can use this dependency:

cronet_http:
    git:
      url: https://github.com/dart-lang/http.git
      ref: jni-0.9.2
      path: pkgs/cronet_http

I regenerated the bindings as well, so it will work just fine.

It doesn't crash on my local device or emulator, but I don't know what would be different about the Play Store...

I'll try to upload a cronet example app to play store to see if I get the same error.

lukehutch commented 2 weeks ago

Thanks, awesome, that build saved me a lot of work! I'll replace this dep and see if it fixes the crash.

lukehutch commented 2 weeks ago

While I have your attention -- is all of JNI deprecated in Android, or does jni just use something that is deprecated?:

Note: /home/luke/.pub-cache/hosted/pub.dev/jni-0.9.2/android/src/main/java/com/github/dart_lang/jni/JniPlugin.java uses or overrides a deprecated API.

HosseinYousefi commented 2 weeks ago

Thanks, awesome, that build saved me a lot of work! I'll replace this dep and see if it fixes the crash.

The PR is here: https://github.com/dart-lang/http/pull/1198 Once cronet_http: 1.2.1 gets released, you won't have to use a git dependency.

While I have your attention -- is all of JNI deprecated in Android, or does jni just use something that is deprecated?:

Haha, I just had this conversation here: https://github.com/dart-lang/http/pull/1198#issuecomment-2114831051

It's not about JNI, it's something related to the way Flutter plugins work. I thought I have suppressed the warning, so users don't get confused by this message. However it seems that simply importing the deprecated class triggers the warning, so I should use the fully qualified name of the package instead of importing it.

You can safely ignore it, and I'll hide the warning in the next release of package:jni.

HosseinYousefi commented 1 week ago

Any updates on this? cronet_http: 1.2.1 is also released.

lukehutch commented 1 week ago

Hi, I have been stuck on iOS build issues, so I haven't uploaded my Android app again to the Play store (and even back when I was uploading periodically, I only saw this specific crash once), but it's possible that one of the recent crash fixes in jni is the reason for the crash I experienced too, so it may be fixed. You can go ahead and close this if you want, and I'll comment on it again if I see the crash again.

You presumably know a lot about JNI already, but I figured I'd send you this link to a JNI project I created a while back. I found I had to very carefully check every incoming parameter (including ensuring that the user is not trying to call a static method in a non-static way or vice versa, etc.), and the thrown status after every JNI call, to avoid crashes. It's a very finicky API. There may be some safety mechanisms that you can see in this code that would be useful for your jni package too, to protect against future possibilities of crashes.

https://github.com/toolfactory/narcissus/blob/main/src/main/c/narcissus.c

HosseinYousefi commented 1 week ago

Thanks for sharing. We do similar things, but instead of using C macros, it's a Dart script that generates a "safer" C API that wraps JNI.

Closing this for now, let me know if your problem persists.

lukehutch commented 3 days ago

Hi @HosseinYousefi -- with latest jni, I am now getting the same exact crash on Android:

https://github.com/flutter/flutter/issues/149229

Can you please re-open this issue?

It seems that jniEnv is not initialized when GetApplicationContext is called:

https://github.com/dart-lang/native/blob/1549dc850a7498954d68e5cdb5583789c37165a4/pkgs/jni/src/dartjni.c#L116-L119

This also seems to be a race condition, because it is only triggered in certain builds or certain execution environments.

So this bascially means that GetApplicationContext is being called before Java_com_github_dart_1lang_jni_JniPlugin_initializeJni.