Repository navigation
android: Correct the JNI helper docs - #40
Open
Bombatomica64 wants to merge 1 commit into
Open
Bombatomica64 wants to merge 1 commit into
Bombatomica64 wants to merge 1 commit into
Conversation
with_env claimed the thread is detached when the closure returns. jni 0.22's attach_current_thread attaches for good, runs the closure in its own local frame, and turns an exception still pending at the end into the returned error, replacing the closure's result. The doc now says so. obtain_env, an alias of with_env with no callers, is deprecated. The unsafe blocks in the JNI helpers, the JNI exports and the document picker gain SAFETY comments, and a stale comment about dropping a JObject (it has no Drop in jni 0.22) goes. Part of longbridge#31. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #31 (PR 1 of the proposed list). Comments and one deprecation only; no behaviour change.
Problem
with_envdoc says the thread is detached again when the closure returns. In jni 0.22,JavaVM::attach_current_threadattaches the thread for good (until it exits) and runs the closure in its own local frame. It also clears a Java exception still pending at the end and returns it as the error, in place of the closure's own result. That last part matters to callers, and the doc did not mention it.obtain_envis an alias ofwith_envwith no callers in the crate.unsafeblocks in the JNI helpers and exports had noSAFETYcomment, and one comment innativeOnDeepLinktalked about not dropping aJObject, which has noDropin jni 0.22.Change
with_env: the doc now describes what jni 0.22 does: the thread stays attached, the closure gets its own local frame, and an exception still pending at the end becomes the returned error.obtain_envis marked#[deprecated(note = "use `with_env`")]. It is still public, so nothing breaks.SAFETYcomments on theunsafeblocks injava_vm_safe,find_app_class,nativeIme,nativeOnDeepLink,nativeMediaActionanddocument_picker::pick_files. The staleJObjectcomment is removed.activity()andget_string()don't get aSAFETYcomment here, because what they rely on is not guaranteed yet. #31 fixes both in later PRs:get_stringchecks the type in PR 2, andactivity()keeps its reference alive under the lock in PR 3.Testing
Run on this branch with the Android NDK 27, target
aarch64-linux-android:cargo fmt --all -- --checkcargo check --target aarch64-linux-android --all-targets, pluscargo checkofexample/cargo clippy --target aarch64-linux-android: the same 8 warnings as onmain, none newcargo test --lib: 46 passed🤖 Generated with Claude Code