From 44bdb8b5a1796610283918786018189b58dbfdc7 Mon Sep 17 00:00:00 2001 From: Robert Bragg Date: Thu, 12 Mar 2026 21:44:21 +0000 Subject: [PATCH] Return Application AssetManager from AndroidApp::asset_manager This makes sure that the `AssetManager` we return from `AndroidApp::asset_manager` can be retained with a static lifetime and never become a wrapper for an invalid pointer. The key change here is that we now return the Application AssetManager (i.e. from Application.getAssets()) instead of the Activity AssetManager. Theoretically there could be some applications that could associate an Activity AssetManager with unique resources but that's not expected to be common (and at least no expected to affect anyone currently using `AndroidApp::asset_manager`). As part of the `APP_ONCE` initialization in `init_android_main_thread` we now get a global reference to the Application AssetManager and get the corresponding AAssetManager that we can trust will be valid for the lifetime of the process since we leak the global reference. Note: The Application `AssetManager` is logically a process-wide resource and so the leaked global is just a technical formality to ensure it can't be garbage collected, but that's assumed to be redundant. Note: If anyone _strictly_ needs the `Activity` `AssetManager` then they could at least resort to calling `Activity.getAssets()` via JNI manually, but perhaps we can later consider adding a separate `AndroidApp::activity_asset_manager()` that will pair an `AAssetManager` pointer with a JNI global reference to ensure the pointer remains valid. Fixes #161 --- android-activity/CHANGELOG.md | 8 +++- android-activity/src/game_activity/mod.rs | 40 +++++++++++------ android-activity/src/lib.rs | 23 +++++++++- android-activity/src/native_activity/glue.rs | 13 ++++-- android-activity/src/native_activity/mod.rs | 21 ++++++--- android-activity/src/util.rs | 46 ++++++++++++++++++-- 6 files changed, 120 insertions(+), 31 deletions(-) diff --git a/android-activity/CHANGELOG.md b/android-activity/CHANGELOG.md index 47b05ad..71e1a1d 100644 --- a/android-activity/CHANGELOG.md +++ b/android-activity/CHANGELOG.md @@ -8,12 +8,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- input: TextInputAction enum representing action button types on soft keyboards. -- input: InputEvent::TextAction event for handling action button presses from soft keyboards. +- input: `TextInputAction` enum representing action button types on soft keyboards. ([#216](https://github.com/rust-mobile/android-activity/pull/216)) +- input: `InputEvent::TextAction` event for handling action button presses from soft keyboards. ([#216](https://github.com/rust-mobile/android-activity/pull/216)) - The `ndk` and `ndk-sys` crates are now re-exported under `android_activity::ndk` and `android_activity::ndk_sys` ([#194](https://github.com/rust-mobile/android-activity/pull/194)) - `AndroidApp::java_main_looper()` gives access to the `ALooper` for the Java main / UI thread ([#198](https://github.com/rust-mobile/android-activity/pull/198)) - `AndroidApp::run_on_java_main_thread()` can be used to run boxed closures on the Java main / UI thread ([#232](https://github.com/rust-mobile/android-activity/pull/232)) +### Fixed + +- *Safety* `AndroidApp::asset_manager()` returns an `AssetManager` that has a safe `'static` lifetime that's not invalidated when `android_main()` returns [#233](https://github.com/rust-mobile/android-activity/pull/233) + ### Changed - rust-version bumped to 1.85.0 ([#193](https://github.com/rust-mobile/android-activity/pull/193), [#219](https://github.com/rust-mobile/android-activity/pull/219)) - GameActivity updated to 4.0.0 (requires the corresponding 4.0.0 `.aar` release from Google) ([#191](https://github.com/rust-mobile/android-activity/pull/191)) diff --git a/android-activity/src/game_activity/mod.rs b/android-activity/src/game_activity/mod.rs index 67d6525..92b0226 100644 --- a/android-activity/src/game_activity/mod.rs +++ b/android-activity/src/game_activity/mod.rs @@ -111,6 +111,7 @@ impl AndroidApp { pub(crate) fn new( ptr: NonNull, jvm: jni::JavaVM, + app_asset_manager: AssetManager, main_looper: ndk::looper::ForeignLooper, main_callbacks: MainCallbacks, ) -> Self { @@ -137,6 +138,7 @@ impl AndroidApp { main_callbacks, key_maps: Mutex::new(HashMap::new()), input_receiver: Mutex::new(None), + app_asset_manager, })), }) }) @@ -303,6 +305,14 @@ pub struct AndroidAppInner { /// InputReceiver reference which we track to ensure /// we don't hand out more than one receiver at a time input_receiver: Mutex>>, + + /// An `AAssetManager` wrapper for the `Application` `AssetManager` + /// Note: `AAssetManager_fromJava` specifies that the pointer is only valid + /// while we hold a global reference to the `AssetManager` Java object + /// to ensure it is not garbage collected. This AssetManager comes from + /// a OnceLock initialization that leaks a single global JNI reference + /// to guarantee that it remains valid for the lifetime of the process. + app_asset_manager: AssetManager, } impl AndroidAppInner { @@ -652,11 +662,11 @@ impl AndroidAppInner { } pub fn asset_manager(&self) -> AssetManager { - unsafe { - let app_ptr = self.native_app.as_ptr(); - let am_ptr = NonNull::new_unchecked((*(*app_ptr).activity).assetManager); - AssetManager::from_ptr(am_ptr) - } + // Safety: While constructing the AndroidApp we do a OnceLock initialization + // where we get the Application AssetManager and leak a single global JNI + // reference that guarantees it will not be garbage collected, so we can + // safely return the corresponding AAssetManager here. + unsafe { AssetManager::from_ptr(self.app_asset_manager.ptr()) } } pub(crate) fn input_events_receiver(&self) -> InternalResult> { @@ -1023,20 +1033,22 @@ pub unsafe extern "C" fn _rust_glue_entry(native_app: *mut ffi::android_app) { // SAFETY: We know jni_activity is a valid JNI global ref to an Activity instance let jni_activity = unsafe { env.as_cast_raw::>(&jni_activity)? }; - let main_callbacks = match init_android_main_thread(&jvm, &jni_activity, &main_looper) { - Ok(main_callbacks) => main_callbacks, - Err(err) => { - eprintln!( - "Failed to name Java thread and set thread context class loader: {err}" - ); - return Err(err); - } - }; + let (app_asset_manager, main_callbacks) = + match init_android_main_thread(&jvm, &jni_activity, &main_looper) { + Ok((asset_manager, main_callbacks)) => (asset_manager, main_callbacks), + Err(err) => { + eprintln!( + "Failed to name Java thread and set thread context class loader: {err}" + ); + return Err(err); + } + }; unsafe { let app = AndroidApp::new( NonNull::new(native_app).unwrap(), jvm.clone(), + app_asset_manager, main_looper, main_callbacks, ); diff --git a/android-activity/src/lib.rs b/android-activity/src/lib.rs index e6f7a56..b5b0366 100644 --- a/android-activity/src/lib.rs +++ b/android-activity/src/lib.rs @@ -783,9 +783,28 @@ impl AndroidApp { self.inner.read().unwrap().content_rect() } - /// Queries the Asset Manager instance for the application. + /// Returns the `AssetManager` for the application's `Application` context. /// - /// Use this to access binary assets bundled inside your application's .apk file. + /// Use this to access raw files bundled in the application's .apk file. + /// + /// This is an `Application`-scoped asset manager, not an `Activity`-scoped + /// one. In normal usage those behave the same for packaged assets, so this + /// is usually the correct API to use. + /// + /// In uncommon cases, an `Activity` may have a context-specific + /// asset/resource view that differs from the `Application` context. If you + /// specifically need the current `Activity`'s `AssetManager`, obtain the + /// `Activity` via [`AndroidApp::activity_as_ptr`] and call `getAssets()` + /// through JNI. + /// + /// The returned `AssetManager` has a `'static` lifetime and remains valid + /// across `Activity` recreation, including when `android_main()` is + /// re-entered. + /// + /// **Beware**: If you consider accessing the `Activity` context's + /// `AssetManager` through JNI you must keep the `AssetManager` alive via a + /// global reference before accessing the ndk `AAssetManager` and + /// `ndk::asset::AssetManager` does not currently handle this for you. pub fn asset_manager(&self) -> AssetManager { self.inner.read().unwrap().asset_manager() } diff --git a/android-activity/src/native_activity/glue.rs b/android-activity/src/native_activity/glue.rs index 517928f..5735fce 100644 --- a/android-activity/src/native_activity/glue.rs +++ b/android-activity/src/native_activity/glue.rs @@ -898,9 +898,9 @@ fn rust_glue_entry( // SAFETY: We know jni_activity is a valid JNI global ref to an Activity instance let jni_activity = unsafe { env.as_cast_raw::>(&jni_activity)? }; - let main_callbacks = + let (app_asset_manager, main_callbacks) = match init_android_main_thread(&jvm, &jni_activity, &main_looper) { - Ok(callbacks) => callbacks, + Ok((asset_manager, callbacks)) => (asset_manager, callbacks), Err(err) => { eprintln!( "Failed to name Java thread and set thread context class loader: {err}" @@ -909,8 +909,13 @@ fn rust_glue_entry( } }; - let app = - AndroidApp::new(rust_glue.clone(), jvm.clone(), main_looper, main_callbacks); + let app = AndroidApp::new( + rust_glue.clone(), + jvm.clone(), + app_asset_manager, + main_looper, + main_callbacks, + ); rust_glue.notify_main_thread_running(); diff --git a/android-activity/src/native_activity/mod.rs b/android-activity/src/native_activity/mod.rs index 1f29836..18c7eef 100644 --- a/android-activity/src/native_activity/mod.rs +++ b/android-activity/src/native_activity/mod.rs @@ -2,7 +2,6 @@ use std::collections::HashMap; use std::marker::PhantomData; use std::panic::AssertUnwindSafe; use std::ptr; -use std::ptr::NonNull; use std::sync::{Arc, Mutex, RwLock, Weak}; use std::time::Duration; @@ -66,6 +65,7 @@ impl AndroidApp { pub(crate) fn new( native_activity: NativeActivityGlue, jvm: JavaVM, + app_asset_manager: AssetManager, main_looper: ndk::looper::ForeignLooper, main_callbacks: MainCallbacks, ) -> Self { @@ -89,6 +89,7 @@ impl AndroidApp { main_callbacks, key_maps: Mutex::new(HashMap::new()), input_receiver: Mutex::new(None), + app_asset_manager, })), }; @@ -139,6 +140,14 @@ pub(crate) struct AndroidAppInner { /// InputReceiver reference which we track to ensure /// we don't hand out more than one receiver at a time input_receiver: Mutex>>, + + /// An `AAssetManager` wrapper for the `Application` `AssetManager` + /// Note: `AAssetManager_fromJava` specifies that the pointer is only valid + /// while we hold a global reference to the `AssetManager` Java object + /// to ensure it is not garbage collected. This AssetManager comes from + /// a OnceLock initialization that leaks a single global JNI reference + /// to guarantee that it remains valid for the lifetime of the process. + app_asset_manager: AssetManager, } impl AndroidAppInner { @@ -315,11 +324,11 @@ impl AndroidAppInner { } pub fn asset_manager(&self) -> AssetManager { - unsafe { - let activity_ptr = self.native_activity.activity; - let am_ptr = NonNull::new_unchecked((*activity_ptr).assetManager); - AssetManager::from_ptr(am_ptr) - } + // Safety: While constructing the AndroidApp we do a OnceLock initialization + // where we get the Application AssetManager and leak a single global JNI + // reference that guarantees it will not be garbage collected, so we can + // safely return the corresponding AAssetManager here. + unsafe { AssetManager::from_ptr(self.app_asset_manager.ptr()) } } pub fn set_window_flags( diff --git a/android-activity/src/util.rs b/android-activity/src/util.rs index 25d1529..ba52883 100644 --- a/android-activity/src/util.rs +++ b/android-activity/src/util.rs @@ -4,6 +4,7 @@ use jni::{ vm::JavaVM, }; use log::{error, Level}; +use ndk::asset::AssetManager; use std::{ ffi::{CStr, CString}, fs::File, @@ -122,6 +123,7 @@ pub(crate) fn abort_on_panic(f: impl FnOnce() -> R) -> R { struct AppState { main_callbacks: MainCallbacks, + app_asset_manager: AssetManager, } static APP_ONCE: OnceLock = OnceLock::new(); @@ -142,6 +144,21 @@ pub(crate) fn get_application<'local, 'any>( Ok(app) } +pub(crate) fn get_assets<'local, 'any>( + env: &mut jni::Env<'local>, + application: &JObject<'any>, +) -> jni::errors::Result> { + let assets_manager = env + .call_method( + application, + jni_str!("getAssets"), + jni_sig!(() -> android.content.res.AssetManager), + &[], + )? + .l()?; + Ok(assets_manager) +} + fn try_init_current_thread(env: &mut jni::Env, activity: &JObject) -> jni::errors::Result<()> { let activity_class = env.get_object_class(activity)?; let class_loader = activity_class.get_class_loader(env)?; @@ -166,11 +183,13 @@ pub(crate) fn init_android_main_thread( vm: &JavaVM, jni_activity: &JObject, java_main_looper: &ndk::looper::ForeignLooper, -) -> jni::errors::Result { +) -> jni::errors::Result<(AssetManager, MainCallbacks)> { vm.with_local_frame(10, |env| -> jni::errors::Result<_> { let app_state = APP_ONCE.get_or_init(|| unsafe { let application = get_application(env, jni_activity).expect("Failed to get Application instance"); + let app_asset_manager = + get_assets(env, &application).expect("Failed to get AssetManager"); let app_global = env .new_global_ref(application) .expect("Failed to create global ref for Application"); @@ -178,17 +197,38 @@ pub(crate) fn init_android_main_thread( let app_global = app_global.into_raw(); ndk_context::initialize_android_context(vm.get_raw().cast(), app_global.cast()); + let asset_manager_global = env + .new_global_ref(app_asset_manager) + .expect("Failed to create global ref for AssetManager"); + // Make sure we don't delete the global reference via Drop because + // the AAssetManager pointer will only be valid while we can + // guarantee that the Java AssetManager is not garbage collected + let asset_manager_global = asset_manager_global.into_raw(); + let asset_manager_ptr = + ndk_sys::AAssetManager_fromJava(env.get_raw() as _, asset_manager_global as _); + assert_ne!( + asset_manager_ptr, + std::ptr::null_mut(), + "Failed to get Application AAssetManager" + ); + let app_asset_manager = + AssetManager::from_ptr(std::ptr::NonNull::new(asset_manager_ptr).unwrap()); + let main_callbacks = MainCallbacks::new(java_main_looper); - AppState { main_callbacks } + AppState { + main_callbacks, + app_asset_manager, + } }); if let Err(err) = try_init_current_thread(env, jni_activity) { eprintln!("Failed to initialize Java thread state: {:?}", err); } + let asset_manager = unsafe { AssetManager::from_ptr(app_state.app_asset_manager.ptr()) }; let main_callbacks = app_state.main_callbacks.clone(); - Ok(main_callbacks) + Ok((asset_manager, main_callbacks)) }) }