Repository navigation
Proposal: rework the Android JNI layer (Activity lifetime, exceptions, UI thread), as small PRs #31
Description
Hi, while building a host-driven test app on gpui-mobile (and the Android PRs #24, #25, #29, #30), I audited how the crate calls into Java: src/android/jni.rs, the other core files, and the 30 packages/*/android.rs. I'd like to propose a set of small fixes, and ask whether you'd welcome them before sending any PRs.
I first did all of it on my fork, so it could be tested as a whole: Bombatomica64#1, 12 commits. It runs on a OnePlus CPH2581 (Android 16) through the host entry point. I'd like to propose it here as separate PRs, each reviewable on its own, in whatever order or subset you prefer.
What the audit found
- Host Activity lifetime.
activity()wraps the raw global reference after releasing theHOST_ACTIVITYlock, and only one previous Activity is kept. Two recreations during one JNI call can delete a reference that call is still using. Also, a finished Activity stays "current" (used as a Context, and kept alive) until the nextonCreate. - Java exceptions. Many calls convert errors with
map_err(to_string)and leave the exception pending. jni 0.22 then refuses every later call in the same frame, and the error only saysJavaException. Other sites clear exceptions by hand, so the Java message is lost, or treat every exception as a normal result: e.g.startActivityfailing for any reason reads as "no app can open this". - UI thread.
set_system_chrome(Windowcalls) andupdate_ime_position(updateCursorAnchorInfowith a View) run on the render thread. The chrome code already notes it can deadlock. - Class lookup cost.
find_app_classasks for the class loader and loads the class again on every call, about 110 call sites in the packages. It also needs an Activity. - Smaller items.
- The
with_envdoc says the thread is detached on return; jni 0.22 attaches it permanently. get_stringreads any object as aStringwithout checking its type.- Three packages (clipboard, deeplink, in_app_review) call Java helpers that only forward to framework APIs, so a host that didn't copy those files gets a
loadClasserror at runtime.
- The
Proposed PRs
Each one builds on its own and changes no public API, except where noted.
| # | PR | Size (approx.) |
|---|---|---|
| 1 | Docs: correct with_env; SAFETY comments; deprecate the unused obtain_env |
small |
| 2 | or_clear / or_catch helpers that clear a pending exception and return its toString() (or_catch treats one exception class as an expected outcome); get_string checks the type; applied to the core files |
medium |
| 3 | Host Activity registry: activity() returns a local reference, taken under the lock, to the newest Activity that is not finishing or destroyed. New application_context(). activity() takes &mut Env (every caller already passes one); activity_as_ptr() is deprecated |
medium |
| 4 | Cache the app class loader (from the application context) and the classes find_app_class loads |
small |
| 5 | run_on_ui_thread through the UI looper and an eventfd (NDK only, no Java), with a latest-wins variant; set_system_chrome and update_ime_position use it. The android-activity path keeps today's behaviour |
medium |
| 6 | The same exception handling in the platform packages (could be split into 2–3 groups) | medium–large, mechanical |
| 7 | Application context in packages that only need a Context | small |
| 8 | clipboard / deeplink / in_app_review: call the framework from Rust and drop their Java helpers, one PR each | small each |
Not included: behaviour issues in individual packages that the audit also found, such as unbounded waits in the pickers, auth and permissions; webview controls that act on a different WebView; and content:// URIs reported as file paths. I can open separate issues for those if useful.
Questions
- Are you interested in this, and in which order would you like it? 2 and 3 fix real crashes or leaks; 1, 4 and 7 are the smallest.
- PR 3 overlaps with
set_host_activityin android: Load the bundled emoji font on the host-driven path #29/android: Follow the system night mode on the host-driven path #30. Should I wait for those, or rebase them onto each other? - PR 8 moves logic from Java into Rust JNI. Do you prefer that (fewer files for hosts to package), or keeping Java helpers?
Thanks for reading. Happy to adjust any of this.
Activity