upgrade jni v0.21

[flisky]

~jni v0.19 doesn't work for me.~

~I use tokio multi threads runtime, and our app is killed by system -~

e.android.debug: java_vm_ext.cc:594] JNI DETECTED ERROR IN APPLICATION: a thread (tid 5917 is making JNI calls without being attached
e.android.debug: java_vm_ext.cc:594]     in call to PushLocalFrame

~It turns out java_vm::get_env() goes null even after calling AttachCurrentThread in a few threads, while other threads are okay.~ ~I have no idea. However, upgrade to jni v0.21.0 eliminates the problem for me.~

fixes #22

[complexspaces]

Thanks for the PR! It looks like the changes here aren't currently compiling for Android. Once CI is passing I'm happy to take a look through this.

It turns out java_vm::get_env() goes null even after calling AttachCurrentThread in a few threads, while other threads are okay. I have no idea. However, upgrade to jni v0.21.0 eliminates the problem for me.

That's very strange, and definitely something I'll look into on the side. We use this exact same JNI-abstraction inside 1Password's Android app (this code was copy/pasted out) and we've never seen this issue AFAICT. If you have any other JNI code in your app, it might be worth checking if its having a poor interaction with rustls-platform-verifier.

[flisky]

If you have any other JNI code in your app, it might be worth checking if its having a poor interaction with rustls-platform-verifier.

We're using mozilla/uniffi which uses JNA actually. We have to configure JNI environment mannually, and https://github.com/mozilla/uniffi-rs/issues/1778#issuecomment-1807979746 inspires us.

[complexspaces]

It would be great if jni could make another release before this merges. I don't want to push a very outdated version of windows-sys into everyone's dependency trees.

[complexspaces]

It turns out java_vm::get_env() goes null even after calling AttachCurrentThread in a few threads, while other threads are okay. I have no idea. However, upgrade to jni v0.21.0 eliminates the problem for me.

We also use a multi-threaded Tokio runtime at work and haven't encountered this, so I am curious where the difference is.

@flisky Are you able to/do you mind sharing your Android context initialization code? While looking at the SO post in your linked GitHub comment, some things stood out to me as strange. Notably its unclear why they are wrangling the list of JVMs and current thread instead of passing an environment from Kotlin/Java and using JniEnv::get_java_vm (which calls GetJavaVM) instead. You can see an example of this here for this crate's tests.

[flisky]

I create a demo project for this, and you can take a look at https://github.com/flisky/jnidemo/blob/main/src/android.rs Unfortunately, I cannot reproduce with that project. And I'll tweak it in next few days.

[flisky]

Hey @complexspaces, I'm back on this issue, and I finally find the root case.

During several days debugging, it turns out jna will also attach & detach thread if current thread is not attached.

We're usinguniffi callback to log tracing events, and the race happens:

1. logging start (jna attached)
2. verifier start (jni attached )
3. logging finished (jna detached)
4. verifying (jni calling).

Now, we're calling attach_current_thread_permanently manully when https://docs.rs/tokio/latest/tokio/runtime/struct.Builder.html#method.on_thread_start, and make sure jni's logging disabled.

Thanks this bug we find out something to optimized. Hope it help:)

(By the way, jna & jni can be mixed used, because java loads native library only once.)

[cpu]

I'm not sure I understand the current state of this PR? Are changes required? Is the update no longer a priority since a workaround was discovered?

[flisky]

I think it's waiting for upstreaming dependency coordination: jni-rs/jni-rs#513, and yes, it's not a priority.
We could revisit it when upsteam goes ready.