diff --git a/Cargo.toml b/Cargo.toml index c1f46d3..5673fe4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,4 +3,3 @@ resolver = "2" members = ["android-activity"] exclude = ["examples"] - diff --git a/android-activity/Cargo.toml b/android-activity/Cargo.toml index d8050c7..dd9f12a 100644 --- a/android-activity/Cargo.toml +++ b/android-activity/Cargo.toml @@ -33,9 +33,9 @@ api-level-33 = ["api-level-30", "ndk/api-level-33"] [dependencies] log = "0.4" -jni-sys = "0.3" cesu8 = "1" -jni = "0.21" +jni = "0.22" +jni-sys = "0.4.1" ndk-sys = "0.6.0" ndk = { version = "0.9.0", default-features = false } ndk-context = "0.1.1" diff --git a/android-activity/src/error.rs b/android-activity/src/error.rs index 7193f5a..8ba182d 100644 --- a/android-activity/src/error.rs +++ b/android-activity/src/error.rs @@ -27,6 +27,10 @@ pub(crate) enum InternalAppError { JniError(jni::errors::JniError), #[error("A Java Exception was thrown via a JNI method call")] JniException(String), + // For internal errors that don't lead to a Java exception but are + // still JNI related. + #[error("A bad argument was passed to a JNI method: {0}")] + JniBadArgument(String), #[error("A Java VM error")] JvmError(jni::errors::Error), #[error("Input unavailable")] @@ -51,6 +55,7 @@ impl From for AppError { match value { InternalAppError::JniError(err) => AppError::JavaError(err.to_string()), InternalAppError::JniException(msg) => AppError::JavaError(msg), + InternalAppError::JniBadArgument(msg) => AppError::JavaError(msg), InternalAppError::JvmError(err) => AppError::JavaError(err.to_string()), InternalAppError::InputUnavailable => AppError::InputUnavailable, } diff --git a/android-activity/src/game_activity/ffi.rs b/android-activity/src/game_activity/ffi.rs index 51063ff..576d489 100644 --- a/android-activity/src/game_activity/ffi.rs +++ b/android-activity/src/game_activity/ffi.rs @@ -12,7 +12,7 @@ #![allow(deref_nullptr)] #![allow(dead_code)] -use jni_sys::*; +use jni::sys::*; use libc::{pthread_cond_t, pthread_mutex_t, pthread_t}; use ndk_sys::{AAssetManager, AConfiguration, ALooper, ALooper_callbackFunc, ANativeWindow, ARect}; diff --git a/android-activity/src/game_activity/mod.rs b/android-activity/src/game_activity/mod.rs index 129175d..bd14461 100644 --- a/android-activity/src/game_activity/mod.rs +++ b/android-activity/src/game_activity/mod.rs @@ -8,10 +8,12 @@ use std::sync::Weak; use std::sync::{Arc, Mutex, RwLock}; use std::time::Duration; +use jni::objects::JObject; +use jni::refs::Global; use libc::c_void; use log::{error, trace}; -use jni_sys::*; +use jni::sys::*; use ndk_sys::ALooper_wake; use ndk_sys::{ALooper, ALooper_pollAll}; @@ -21,9 +23,11 @@ use ndk::configuration::Configuration; use ndk::native_window::NativeWindow; use crate::error::InternalResult; -use crate::input::{Axis, KeyCharacterMap, KeyCharacterMapBinding}; -use crate::jni_utils::{self, CloneJavaVM}; -use crate::util::{abort_on_panic, forward_stdio_to_logcat, log_panic, try_get_path_from_ptr}; +use crate::input::{device_key_character_map, Axis, KeyCharacterMap}; +use crate::util::{ + abort_on_panic, forward_stdio_to_logcat, init_android_main_thread, log_panic, + try_get_path_from_ptr, +}; use crate::{ AndroidApp, ConfigurationRef, InputStatus, MainEvent, PollEvent, Rect, WindowManagerFlags, }; @@ -119,32 +123,31 @@ impl AndroidAppWaker { } impl AndroidApp { - pub(crate) unsafe fn from_ptr(ptr: NonNull, jvm: CloneJavaVM) -> Self { - let mut env = jvm.get_env().unwrap(); // We attach to the thread before creating the AndroidApp + pub(crate) unsafe fn from_ptr(ptr: NonNull, jvm: jni::JavaVM) -> Self { + // We attach to the thread before creating the AndroidApp + jvm.with_local_frame(10, |env| -> jni::errors::Result<_> { + if let Err(err) = crate::input::jni_init(env) { + panic!("Failed to init JNI bindings: {err:?}"); + }; - let key_map_binding = match KeyCharacterMapBinding::new(&mut env) { - Ok(b) => b, - Err(err) => { - panic!("Failed to create KeyCharacterMap JNI bindings: {err:?}"); - } - }; + // Note: we don't use from_ptr since we don't own the android_app.config + // and need to keep in mind that the Drop handler is going to call + // AConfiguration_delete() + let config = + Configuration::clone_from_ptr(NonNull::new_unchecked((*ptr.as_ptr()).config)); - // Note: we don't use from_ptr since we don't own the android_app.config - // and need to keep in mind that the Drop handler is going to call - // AConfiguration_delete() - let config = Configuration::clone_from_ptr(NonNull::new_unchecked((*ptr.as_ptr()).config)); - - Self { - inner: Arc::new(RwLock::new(AndroidAppInner { - jvm, - native_app: NativeAppGlue { ptr }, - config: ConfigurationRef::new(config), - native_window: Default::default(), - key_map_binding: Arc::new(key_map_binding), - key_maps: Mutex::new(HashMap::new()), - input_receiver: Mutex::new(None), - })), - } + Ok(Self { + inner: Arc::new(RwLock::new(AndroidAppInner { + jvm: jvm.clone(), + native_app: NativeAppGlue { ptr }, + config: ConfigurationRef::new(config), + native_window: Default::default(), + key_maps: Mutex::new(HashMap::new()), + input_receiver: Mutex::new(None), + })), + }) + }) + .expect("Failed to create AndroidApp instance") } } @@ -251,14 +254,11 @@ impl NativeAppGlue { #[derive(Debug)] pub struct AndroidAppInner { - pub(crate) jvm: CloneJavaVM, + pub(crate) jvm: jni::JavaVM, native_app: NativeAppGlue, config: ConfigurationRef, native_window: RwLock>, - /// Shared JNI bindings for the `KeyCharacterMap` class - key_map_binding: Arc, - /// A table of `KeyCharacterMap`s per `InputDevice` ID /// these are used to be able to map key presses to unicode /// characters @@ -533,11 +533,7 @@ impl AndroidAppInner { let key_map = match guard.entry(device_id) { std::collections::hash_map::Entry::Occupied(occupied) => occupied.get().clone(), std::collections::hash_map::Entry::Vacant(vacant) => { - let character_map = jni_utils::device_key_character_map( - self.jvm.clone(), - self.key_map_binding.clone(), - device_id, - )?; + let character_map = device_key_character_map(self.jvm.clone(), device_id)?; vacant.insert(character_map.clone()); character_map } @@ -876,7 +872,7 @@ pub unsafe extern "C" fn Java_com_google_androidgamesdk_GameActivity_initializeN jasset_mgr: jobject, saved_state: jbyteArray, java_config: jobject, -) -> jni_sys::jlong { +) -> jlong { Java_com_google_androidgamesdk_GameActivity_initializeNativeCode_C( env, java_game_activity, @@ -910,53 +906,56 @@ pub unsafe extern "C" fn _rust_glue_entry(native_app: *mut ffi::android_app) { abort_on_panic(|| { let _join_log_forwarder = forward_stdio_to_logcat(); - let jvm = unsafe { + let (jvm, jni_activity) = unsafe { let jvm = (*(*native_app).activity).vm; let activity: jobject = (*(*native_app).activity).javaGameActivity; ndk_context::initialize_android_context(jvm.cast(), activity.cast()); - - let jvm = CloneJavaVM::from_raw(jvm).unwrap(); - // Since this is a newly spawned thread then the JVM hasn't been attached - // to the thread yet. Attach before calling the applications main function - // so they can safely make JNI calls - jvm.attach_current_thread_permanently().unwrap(); - jvm + (jni::JavaVM::from_raw(jvm), activity) }; + // Note: At this point we can assume jni::JavaVM::singleton is initialized - unsafe { - // Name thread - this needs to happen here after attaching to a JVM thread, - // since that changes the thread name to something like "Thread-2". - let thread_name = std::ffi::CStr::from_bytes_with_nul(b"android_main\0").unwrap(); - libc::pthread_setname_np(libc::pthread_self(), thread_name.as_ptr()); + // Note: the GameActivity implementation will have already attached the main thread to the + // JVM before calling _rust_glue_entry so we don't to set the thread name via + // attach_current_thread_with_config since that won't actually create a new attachment. + // + // Calling .attach_current_thread will ensure that the `jni` crate knows about the + // attachment, as a convenience. + jvm.attach_current_thread(|env| -> jni::errors::Result<()> { + // 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 app = AndroidApp::from_ptr(NonNull::new(native_app).unwrap(), jvm.clone()); + if let Err(err) = init_android_main_thread(&jvm, &jni_activity) { + eprintln!("Failed to name Java thread and set thread context class loader: {err}"); + } - // We want to specifically catch any panic from the application's android_main - // so we can finish + destroy the Activity gracefully via the JVM - catch_unwind(|| { - // XXX: If we were in control of the Java Activity subclass then - // we could potentially run the android_main function via a Java native method - // springboard (e.g. call an Activity subclass method that calls a jni native - // method that then just calls android_main()) that would make sure there was - // a Java frame at the base of our call stack which would then be recognised - // when calling FindClass to lookup a suitable classLoader, instead of - // defaulting to the system loader. Without this then it's difficult for native - // code to look up non-standard Java classes. - android_main(app); - }) - .unwrap_or_else(log_panic); + unsafe { + let app = AndroidApp::from_ptr(NonNull::new(native_app).unwrap(), jvm.clone()); + // We want to specifically catch any panic from the application's android_main + // so we can finish + destroy the Activity gracefully via the JVM + catch_unwind(|| { + // XXX: If we were in control of the Java Activity subclass then + // we could potentially run the android_main function via a Java native method + // springboard (e.g. call an Activity subclass method that calls a jni native + // method that then just calls android_main()) that would make sure there was + // a Java frame at the base of our call stack which would then be recognised + // when calling FindClass to lookup a suitable classLoader, instead of + // defaulting to the system loader. Without this then it's difficult for native + // code to look up non-standard Java classes. + android_main(app); + }) + .unwrap_or_else(log_panic); - // Let JVM know that our Activity can be destroyed before detaching from the JVM - // - // "Note that this method can be called from any thread; it will send a message - // to the main thread of the process where the Java finish call will take place" - ffi::GameActivity_finish((*native_app).activity); + // Let JVM know that our Activity can be destroyed before detaching from the JVM + // + // "Note that this method can be called from any thread; it will send a message + // to the main thread of the process where the Java finish call will take place" + ffi::GameActivity_finish((*native_app).activity); - // This should detach automatically but lets detach explicitly to avoid depending - // on the TLS trickery in `jni-rs` - jvm.detach_current_thread(); + ndk_context::release_android_context(); + } - ndk_context::release_android_context(); - } + Ok(()) + }) + .expect("Failed to attach thread to JVM"); }) } diff --git a/android-activity/src/input/sdk.rs b/android-activity/src/input/sdk.rs index 978ad5a..2d3b2df 100644 --- a/android-activity/src/input/sdk.rs +++ b/android-activity/src/input/sdk.rs @@ -1,21 +1,9 @@ -use std::sync::Arc; +use jni::sys::jint; +use jni::{objects::Global, JavaVM}; -use jni::{ - objects::{GlobalRef, JClass, JMethodID, JObject, JStaticMethodID, JValue}, - signature::{Primitive, ReturnType}, - JNIEnv, -}; -use jni_sys::jint; - -use crate::{ - input::{Keycode, MetaState}, - jni_utils::CloneJavaVM, -}; - -use crate::{ - error::{AppError, InternalAppError}, - jni_utils, -}; +use crate::error::{AppError, InternalAppError, InternalResult}; +use crate::input::{Keycode, MetaState}; +use crate::jni_utils; /// An enum representing the types of keyboards that may generate key events /// @@ -80,157 +68,86 @@ pub enum KeyMapChar { CombiningAccent(char), } -// I've also tried to think here about how to we could potentially automatically -// generate a binding struct like `KeyCharacterMapBinding` with a procmacro and -// so have intentionally limited the `Binding` being a very thin, un-opinionated -// wrapper based on basic JNI types. - -/// Lower-level JNI binding for `KeyCharacterMap` class only holds 'static state -/// and can be shared with an `Arc` ref count. -/// -/// The separation here also neatly helps us separate `InternalAppError` from -/// `AppError` for mapping JNI errors without exposing any `jni-rs` types in the -/// public API. -#[derive(Debug)] -pub(crate) struct KeyCharacterMapBinding { - //vm: JavaVM, - klass: GlobalRef, - get_method_id: JMethodID, - get_dead_char_method_id: JStaticMethodID, - get_keyboard_type_method_id: JMethodID, +jni::bind_java_type! { + pub(crate) AKeyCharacterMap => "android.view.KeyCharacterMap", + methods { + priv fn _get(key_code: jint, meta_state: jint) -> jint, + priv static fn _get_dead_char(accent_char: jint, base_char: jint) -> jint, + priv fn _get_keyboard_type() -> jint, + } } -impl KeyCharacterMapBinding { - pub(crate) fn new(env: &mut JNIEnv) -> Result { - let binding = env.with_local_frame::<_, _, InternalAppError>(10, |env| { - let klass = env.find_class("android/view/KeyCharacterMap")?; // Creates a local ref - Ok(Self { - get_method_id: env.get_method_id(&klass, "get", "(II)I")?, - get_dead_char_method_id: env.get_static_method_id( - &klass, - "getDeadChar", - "(II)I", - )?, - get_keyboard_type_method_id: env.get_method_id(&klass, "getKeyboardType", "()I")?, - klass: env.new_global_ref(&klass)?, - }) - })?; - Ok(binding) - } - - pub fn get<'local>( +impl AKeyCharacterMap<'_> { + pub(crate) fn get<'local>( &self, - env: &'local mut JNIEnv, - key_map: impl AsRef>, + env: &'local mut jni::Env, key_code: jint, meta_state: jint, ) -> Result { - let key_map = key_map.as_ref(); - - // Safety: - // - we know our global `key_map` reference is non-null and valid. - // - we know `get_method_id` remains valid - // - we know that the signature of KeyCharacterMap::get is `(int, int) -> int` - // - we know this won't leak any local references as a side effect - // - // We know it's ok to unwrap the `.i()` value since we explicitly - // specify the return type as `Int` - let unicode = unsafe { - env.call_method_unchecked( - key_map, - self.get_method_id, - ReturnType::Primitive(Primitive::Int), - &[ - JValue::Int(key_code).as_jni(), - JValue::Int(meta_state).as_jni(), - ], - ) - } - .map_err(|err| jni_utils::clear_and_map_exception_to_err(env, err))?; - Ok(unicode.i().unwrap()) + self._get(env, key_code, meta_state) + .map_err(|err| jni_utils::clear_and_map_exception_to_err(env, err)) } - pub fn get_dead_char( - &self, - env: &mut JNIEnv, + pub(crate) fn get_dead_char( + env: &mut jni::Env, accent_char: jint, base_char: jint, ) -> Result { - // Safety: - // - we know `get_dead_char_method_id` remains valid - // - we know that KeyCharacterMap::getDeadKey is a static method - // - we know that the signature of KeyCharacterMap::getDeadKey is `(int, int) -> int` - // - we know this won't leak any local references as a side effect - // - // We know it's ok to unwrap the `.i()` value since we explicitly - // specify the return type as `Int` - - // Urgh, it's pretty terrible that there's no ergonomic/safe way to get a JClass reference from a GlobalRef - // Safety: we don't do anything that would try to delete the JClass as if it were a real local reference - let klass = unsafe { JClass::from_raw(self.klass.as_obj().as_raw()) }; - let unicode = unsafe { - env.call_static_method_unchecked( - &klass, - self.get_dead_char_method_id, - ReturnType::Primitive(Primitive::Int), - &[ - JValue::Int(accent_char).as_jni(), - JValue::Int(base_char).as_jni(), - ], - ) - } - .map_err(|err| jni_utils::clear_and_map_exception_to_err(env, err))?; - Ok(unicode.i().unwrap()) + Self::_get_dead_char(env, accent_char, base_char) + .map_err(|err| jni_utils::clear_and_map_exception_to_err(env, err)) } - pub fn get_keyboard_type<'local>( + pub(crate) fn get_keyboard_type<'local>( &self, - env: &'local mut JNIEnv, - key_map: impl AsRef>, + env: &'local mut jni::Env, ) -> Result { - let key_map = key_map.as_ref(); - - // Safety: - // - we know our global `key_map` reference is non-null and valid. - // - we know `get_keyboard_type_method_id` remains valid - // - we know that the signature of KeyCharacterMap::getKeyboardType is `() -> int` - // - we know this won't leak any local references as a side effect - // - // We know it's ok to unwrap the `.i()` value since we explicitly - // specify the return type as `Int` - Ok(unsafe { - env.call_method_unchecked( - key_map, - self.get_keyboard_type_method_id, - ReturnType::Primitive(Primitive::Int), - &[], - ) - } - .map_err(|err| jni_utils::clear_and_map_exception_to_err(env, err))? - .i() - .unwrap()) + self._get_keyboard_type(env) + .map_err(|err| jni_utils::clear_and_map_exception_to_err(env, err)) } } +jni::bind_java_type! { + rust_type = AInputDevice, + java_type = "android.view.InputDevice", + type_map { + AKeyCharacterMap => "android.view.KeyCharacterMap", + }, + methods { + static fn get_device(id: jint) -> AInputDevice, + fn get_key_character_map() -> AKeyCharacterMap, + } +} + +// Explicitly initialize the JNI bindings so we can get and early, upfront, +// error if something is wrong. +pub fn jni_init(env: &jni::Env) -> jni::errors::Result<()> { + let _ = AKeyCharacterMapAPI::get(env, &Default::default())?; + let _ = AInputDeviceAPI::get(env, &Default::default())?; + Ok(()) +} + /// Describes the keys provided by a keyboard device and their associated labels. -#[derive(Clone, Debug)] +#[derive(Debug)] pub struct KeyCharacterMap { - jvm: CloneJavaVM, - binding: Arc, - key_map: GlobalRef, + jvm: JavaVM, + key_map: Global>, +} +impl Clone for KeyCharacterMap { + fn clone(&self) -> Self { + let jvm = self.jvm.clone(); + jvm.attach_current_thread(|env| -> jni::errors::Result<_> { + Ok(Self { + jvm: jvm.clone(), + key_map: env.new_global_ref(&self.key_map)?, + }) + }) + .expect("Failed to attach thread to JVM and clone key map") + } } impl KeyCharacterMap { - pub(crate) fn new( - jvm: CloneJavaVM, - binding: Arc, - key_map: GlobalRef, - ) -> Self { - Self { - jvm, - binding, - key_map, - } + pub(crate) fn new(jvm: JavaVM, key_map: Global>) -> Self { + Self { jvm, key_map } } /// Gets the Unicode character generated by the specified [`Keycode`] and [`MetaState`] combination. @@ -249,37 +166,33 @@ impl KeyCharacterMap { let key_code = key_code.into(); let meta_state = meta_state.0 as i32; - // Since we expect this API to be called from the `main` thread then we expect to already be - // attached to the JVM - // - // Safety: there's no other JNIEnv in scope so this env can't be used to subvert the mutable - // borrow rules that ensure we can only add local references to the top JNI frame. - let mut env = self.jvm.get_env().map_err(|err| { + let vm = self.jvm.clone(); + vm.attach_current_thread(|env| -> InternalResult<_> { + let unicode = self.key_map.get(env, key_code, meta_state)?; + let unicode = unicode as u32; + + const COMBINING_ACCENT: u32 = 0x80000000; + const COMBINING_ACCENT_MASK: u32 = !COMBINING_ACCENT; + + if unicode == 0 { + Ok(KeyMapChar::None) + } else if unicode & COMBINING_ACCENT == COMBINING_ACCENT { + let accent = unicode & COMBINING_ACCENT_MASK; + // Safety: assumes Android key maps don't contain invalid unicode characters + Ok(KeyMapChar::CombiningAccent(unsafe { + char::from_u32_unchecked(accent) + })) + } else { + // Safety: assumes Android key maps don't contain invalid unicode characters + Ok(KeyMapChar::Unicode(unsafe { + char::from_u32_unchecked(unicode) + })) + } + }) + .map_err(|err| { let err: InternalAppError = err.into(); - err - })?; - let unicode = self - .binding - .get(&mut env, self.key_map.as_obj(), key_code, meta_state)?; - let unicode = unicode as u32; - - const COMBINING_ACCENT: u32 = 0x80000000; - const COMBINING_ACCENT_MASK: u32 = !COMBINING_ACCENT; - - if unicode == 0 { - Ok(KeyMapChar::None) - } else if unicode & COMBINING_ACCENT == COMBINING_ACCENT { - let accent = unicode & COMBINING_ACCENT_MASK; - // Safety: assumes Android key maps don't contain invalid unicode characters - Ok(KeyMapChar::CombiningAccent(unsafe { - char::from_u32_unchecked(accent) - })) - } else { - // Safety: assumes Android key maps don't contain invalid unicode characters - Ok(KeyMapChar::Unicode(unsafe { - char::from_u32_unchecked(unicode) - })) - } + err.into() + }) } /// Get the character that is produced by combining the dead key producing accent with the key producing character c. @@ -295,28 +208,24 @@ impl KeyCharacterMap { accent_char: char, base_char: char, ) -> Result, AppError> { - let accent_char = accent_char as jni_sys::jint; - let base_char = base_char as jni_sys::jint; + let accent_char = accent_char as jni::sys::jint; + let base_char = base_char as jni::sys::jint; - // Since we expect this API to be called from the `main` thread then we expect to already be - // attached to the JVM - // - // Safety: there's no other JNIEnv in scope so this env can't be used to subvert the mutable - // borrow rules that ensure we can only add local references to the top JNI frame. - let mut env = self.jvm.get_env().map_err(|err| { + let vm = self.jvm.clone(); + vm.attach_current_thread(|env| -> InternalResult<_> { + let unicode = AKeyCharacterMap::get_dead_char(env, accent_char, base_char)?; + let unicode = unicode as u32; + + // Safety: assumes Android key maps don't contain invalid unicode characters + Ok(if unicode == 0 { + None + } else { + Some(unsafe { char::from_u32_unchecked(unicode) }) + }) + }) + .map_err(|err| { let err: InternalAppError = err.into(); - err - })?; - let unicode = self - .binding - .get_dead_char(&mut env, accent_char, base_char)?; - let unicode = unicode as u32; - - // Safety: assumes Android key maps don't contain invalid unicode characters - Ok(if unicode == 0 { - None - } else { - Some(unsafe { char::from_u32_unchecked(unicode) }) + err.into() }) } @@ -330,19 +239,49 @@ impl KeyCharacterMap { /// a [`AppError::JavaError`] in case there is a spurious JNI error or an exception /// is caught. pub fn get_keyboard_type(&self) -> Result { - // Since we expect this API to be called from the `main` thread then we expect to already be - // attached to the JVM - // - // Safety: there's no other JNIEnv in scope so this env can't be used to subvert the mutable - // borrow rules that ensure we can only add local references to the top JNI frame. - let mut env = self.jvm.get_env().map_err(|err| { + let vm = self.jvm.clone(); + vm.attach_current_thread(|env| -> InternalResult<_> { + let keyboard_type = self.key_map.get_keyboard_type(env)?; + let keyboard_type = keyboard_type as u32; + Ok(keyboard_type.into()) + }) + .map_err(|err| { let err: InternalAppError = err.into(); - err - })?; - let keyboard_type = self - .binding - .get_keyboard_type(&mut env, self.key_map.as_obj())?; - let keyboard_type = keyboard_type as u32; - Ok(keyboard_type.into()) + err.into() + }) } } + +fn device_key_character_map_with_env( + env: &mut jni::Env<'_>, + device_id: i32, +) -> jni::errors::Result { + let device = AInputDevice::get_device(env, device_id)?; + if device.is_null() { + // This isn't really an error from a JNI perspective but we would only expect + // this to return null for a device ID of zero or an invalid device ID. + log::error!("No input device with id {}", device_id); + return Err(jni::errors::Error::WrongObjectType); + } + let character_map = device.get_key_character_map(env)?; + let character_map = env.new_global_ref(character_map)?; + Ok(KeyCharacterMap::new( + env.get_java_vm().clone(), + character_map, + )) +} + +pub(crate) fn device_key_character_map( + jvm: JavaVM, + device_id: i32, +) -> InternalResult { + jvm.attach_current_thread(|env| { + if device_id == 0 { + return Err(InternalAppError::JniBadArgument( + "Can't get key character map for non-physical device_id 0".into(), + )); + } + device_key_character_map_with_env(env, device_id) + .map_err(|err| jni_utils::clear_and_map_exception_to_err(env, err)) + }) +} diff --git a/android-activity/src/jni_utils.rs b/android-activity/src/jni_utils.rs index dcf6083..eee10d4 100644 --- a/android-activity/src/jni_utils.rs +++ b/android-activity/src/jni_utils.rs @@ -5,46 +5,21 @@ //! //! These utilities help us check + clear exceptions and map them into Rust Errors. -use std::{ops::Deref, sync::Arc}; +use crate::error::InternalAppError; -use jni::{ - objects::{JObject, JString}, - JavaVM, -}; - -use crate::{ - error::{InternalAppError, InternalResult}, - input::{KeyCharacterMap, KeyCharacterMapBinding}, -}; - -// TODO: JavaVM should implement Clone -#[derive(Debug)] -pub(crate) struct CloneJavaVM { - pub jvm: JavaVM, -} -impl Clone for CloneJavaVM { - fn clone(&self) -> Self { - Self { - jvm: unsafe { JavaVM::from_raw(self.jvm.get_java_vm_pointer()).unwrap() }, - } - } -} -impl CloneJavaVM { - pub unsafe fn from_raw(jvm: *mut jni_sys::JavaVM) -> InternalResult { - Ok(Self { - jvm: JavaVM::from_raw(jvm)?, - }) - } -} -unsafe impl Send for CloneJavaVM {} -unsafe impl Sync for CloneJavaVM {} - -impl Deref for CloneJavaVM { - type Target = JavaVM; - - fn deref(&self) -> &Self::Target { - &self.jvm +fn try_get_stack_trace( + env: &mut jni::Env<'_>, + throwable: &jni::objects::JThrowable, +) -> jni::errors::Result { + let stack_trace = throwable.get_stack_trace(env)?; + let len = stack_trace.len(env)?; + let mut trace = String::new(); + for i in 0..len { + let element = stack_trace.get_element(env, i)?; + let element_jstr = element.try_to_string(env)?; + trace.push_str(&format!("{i}: {element_jstr}\n")); } + Ok(trace) } /// Use with `.map_err()` to map `jni::errors::Error::JavaException` into a @@ -55,41 +30,28 @@ impl Deref for CloneJavaVM { /// /// This will also clear the exception pub(crate) fn clear_and_map_exception_to_err( - env: &mut jni::JNIEnv<'_>, + env: &mut jni::Env<'_>, err: jni::errors::Error, ) -> InternalAppError { if matches!(err, jni::errors::Error::JavaException) { let result = env.with_local_frame::<_, _, InternalAppError>(5, |env| { - let e = env.exception_occurred()?; - assert!(!e.is_null()); // should only be called after receiving a JavaException Result - env.exception_clear()?; - - let class = env.get_object_class(&e)?; - //let get_stack_trace_method = env.get_method_id(&class, "getStackTrace", "()[Ljava/lang/StackTraceElement;")?; - let get_message_method = - env.get_method_id(&class, "getMessage", "()Ljava/lang/String;")?; - - let msg = unsafe { - env.call_method_unchecked( - &e, - get_message_method, - jni::signature::ReturnType::Object, - &[], - )? - .l() - .unwrap() + let Some(e) = env.exception_occurred() else { + // should only be called after receiving a JavaException Result + unreachable!("JNI Error was JavaException but no exception was set"); }; - let msg = unsafe { JString::from_raw(JObject::into_raw(msg)) }; - let msg = env.get_string(&msg)?; - let msg: String = msg.into(); - - // TODO: get Java backtrace: - /* - if let JValue::Object(elements) = env.call_method_unchecked(&e, get_stack_trace_method, jni::signature::ReturnType::Array, &[])? { - let elements = env.auto_local(elements); + env.exception_clear(); + let msg = e.get_message(env)?; + let mut msg: String = msg.to_string(); + match try_get_stack_trace(env, &e) { + Ok(stack_trace) => { + msg.push_str("stack trace:\n"); + msg.push_str(&stack_trace); + } + Err(err) => { + msg.push_str(&format!("\nfailed to get stack trace: {err:?}")); + } } - */ Ok(msg) }); @@ -104,48 +66,3 @@ pub(crate) fn clear_and_map_exception_to_err( err.into() } } - -pub(crate) fn device_key_character_map( - jvm: CloneJavaVM, - key_map_binding: Arc, - device_id: i32, -) -> InternalResult { - // Don't really need to 'attach' since this should be called from the app's main thread that - // should already be attached, but the redundancy should be fine - // - // Attach 'permanently' to avoid any chance of detaching the thread from the VM - let mut env = jvm.attach_current_thread_permanently()?; - - // We don't want to accidentally leak any local references while we - // aren't going to be returning from here back to the JVM, to unwind, so - // we make a local frame - let character_map = env.with_local_frame::<_, _, jni::errors::Error>(10, |env| { - let input_device_class = env.find_class("android/view/InputDevice")?; // Creates a local ref - let device = env - .call_static_method( - input_device_class, - "getDevice", - "(I)Landroid/view/InputDevice;", - &[device_id.into()], - )? - .l()?; // Creates a local ref - - let character_map = env - .call_method( - &device, - "getKeyCharacterMap", - "()Landroid/view/KeyCharacterMap;", - &[], - )? - .l()?; - let character_map = env.new_global_ref(character_map)?; - - Ok(character_map) - })?; - - Ok(KeyCharacterMap::new( - jvm.clone(), - key_map_binding, - character_map, - )) -} diff --git a/android-activity/src/lib.rs b/android-activity/src/lib.rs index 7a05078..51c63b8 100644 --- a/android-activity/src/lib.rs +++ b/android-activity/src/lib.rs @@ -566,7 +566,7 @@ impl AndroidApp { /// with the [`jni`] crate (or similar crates) to make JNI calls that bridge /// between native Rust code and Java/Kotlin code running within the JVM. /// - /// If you use the [`jni`] crate you can wrap this as a [`JavaVM`] via: + /// If you use the [`jni`] crate you can could this as a [`JavaVM`] via: /// ```no_run /// # use jni::JavaVM; /// # let app: android_activity::AndroidApp = todo!(); @@ -581,24 +581,28 @@ impl AndroidApp { /// Returns a JNI object reference for this application's JVM `Activity` as a pointer /// - /// If you use the [`jni`] crate you can wrap this as an object reference via: + /// If you use the [`jni`] crate you can cast this as a `JObject` reference via: /// ```no_run /// # use jni::objects::JObject; - /// # let app: android_activity::AndroidApp = todo!(); - /// let activity = unsafe { JObject::from_raw(app.activity_as_ptr().cast()) }; + /// # use jni::refs::Global; + /// # fn use_jni(env: &jni::Env, app: &android_activity::AndroidApp) -> jni::errors::Result<()> { + /// let raw_activity_global = app.activity_as_ptr() as jni::sys::jobject; + /// // SAFETY: The pointer is valid as long as `app` is valid. + /// let activity = unsafe { env.as_cast_raw::>(&raw_activity_global)? }; + /// # Ok(()) } /// ``` /// /// # JNI Safety /// /// Note that the object reference will be a JNI global reference, not a /// local reference and it should not be deleted. Don't wrap the reference - /// in an [`AutoLocal`] which would try to explicitly delete the reference - /// when dropped. Similarly, don't wrap the reference as a [`GlobalRef`] + /// in an [`Auto`] which would try to explicitly delete the reference + /// when dropped. Similarly, don't wrap the reference as a [`Global`] /// which would also try to explicitly delete the reference when dropped. /// /// [`jni`]: https://crates.io/crates/jni - /// [`AutoLocal`]: https://docs.rs/jni/latest/jni/objects/struct.AutoLocal.html - /// [`GlobalRef`]: https://docs.rs/jni/latest/jni/objects/struct.GlobalRef.html + /// [`Auto`]: https://docs.rs/jni/latest/jni/refs/struct.Auto.html + /// [`Global`]: https://docs.rs/jni/latest/jni/refs/struct.Global.html pub fn activity_as_ptr(&self) -> *mut c_void { self.inner.read().unwrap().activity_as_ptr() } @@ -855,6 +859,9 @@ impl AndroidApp { /// Since this API needs to use JNI internally to call into the Android JVM it may return /// a [`error::AppError::JavaError`] in case there is a spurious JNI error or an exception /// is caught. + /// + /// This API should not be called with a `device_id` of `0`, since that indicates a non-physical + /// device and will result in a [`error::AppError::JavaError`]. pub fn device_key_character_map(&self, device_id: i32) -> Result { Ok(self .inner diff --git a/android-activity/src/native_activity/glue.rs b/android-activity/src/native_activity/glue.rs index 82672a5..67cc6a8 100644 --- a/android-activity/src/native_activity/glue.rs +++ b/android-activity/src/native_activity/glue.rs @@ -9,11 +9,11 @@ use std::{ sync::{Arc, Condvar, Mutex, Weak}, }; +use jni::{objects::JObject, refs::Global, vm::AttachConfig}; use ndk::{configuration::Configuration, input_queue::InputQueue, native_window::NativeWindow}; use crate::{ - jni_utils::CloneJavaVM, - util::{abort_on_panic, forward_stdio_to_logcat, log_panic}, + util::{abort_on_panic, forward_stdio_to_logcat, init_android_main_thread, log_panic}, ConfigurationRef, }; @@ -840,11 +840,9 @@ extern "C" fn ANativeActivity_onCreate( abort_on_panic(|| { let _join_log_forwarder = forward_stdio_to_logcat(); - log::trace!( + eprintln!( "Creating: {:p}, saved_state = {:p}, save_state_size = {}", - activity, - saved_state, - saved_state_size + activity, saved_state, saved_state_size ); // Conceptually we associate a glue reference with the JVM main thread, and another @@ -858,60 +856,7 @@ extern "C" fn ANativeActivity_onCreate( // Note: we drop the thread handle which will detach the thread std::thread::spawn(move || { let activity: *mut ndk_sys::ANativeActivity = activity_ptr as *mut _; - - let jvm = abort_on_panic(|| unsafe { - let na = activity; - let jvm: *mut jni_sys::JavaVM = (*na).vm; - let activity = (*na).clazz; // Completely bogus name; this is the _instance_ not class pointer - ndk_context::initialize_android_context(jvm.cast(), activity.cast()); - - let jvm = CloneJavaVM::from_raw(jvm).unwrap(); - // Since this is a newly spawned thread then the JVM hasn't been attached - // to the thread yet. Attach before calling the applications main function - // so they can safely make JNI calls - jvm.attach_current_thread_permanently().unwrap(); - jvm - }); - - let app = AndroidApp::new(rust_glue.clone(), jvm.clone()); - - rust_glue.notify_main_thread_running(); - - unsafe { - // Name thread - this needs to happen here after attaching to a JVM thread, - // since that changes the thread name to something like "Thread-2". - let thread_name = std::ffi::CStr::from_bytes_with_nul(b"android_main\0").unwrap(); - libc::pthread_setname_np(libc::pthread_self(), thread_name.as_ptr()); - - // We want to specifically catch any panic from the application's android_main - // so we can finish + destroy the Activity gracefully via the JVM - catch_unwind(|| { - // XXX: If we were in control of the Java Activity subclass then - // we could potentially run the android_main function via a Java native method - // springboard (e.g. call an Activity subclass method that calls a jni native - // method that then just calls android_main()) that would make sure there was - // a Java frame at the base of our call stack which would then be recognised - // when calling FindClass to lookup a suitable classLoader, instead of - // defaulting to the system loader. Without this then it's difficult for native - // code to look up non-standard Java classes. - android_main(app); - }) - .unwrap_or_else(log_panic); - - // Let JVM know that our Activity can be destroyed before detaching from the JVM - // - // "Note that this method can be called from any thread; it will send a message - // to the main thread of the process where the Java finish call will take place" - ndk_sys::ANativeActivity_finish(activity); - - // This should detach automatically but lets detach explicitly to avoid depending - // on the TLS trickery in `jni-rs` - jvm.detach_current_thread(); - - ndk_context::release_android_context(); - } - - rust_glue.notify_main_thread_stopped_running(); + rust_glue_entry(rust_glue, activity); }); // Wait for thread to start. @@ -924,3 +869,69 @@ extern "C" fn ANativeActivity_onCreate( } }) } + +fn rust_glue_entry(rust_glue: NativeActivityGlue, activity: *mut ndk_sys::ANativeActivity) { + abort_on_panic(|| { + let (jvm, jni_activity) = unsafe { + let jvm: *mut jni::sys::JavaVM = (*activity).vm.cast(); + let jni_activity: jni::sys::jobject = (*activity).clazz as _; // Completely bogus name; this is the _instance_ not class pointer + ndk_context::initialize_android_context(jvm.cast(), jni_activity.cast()); + (jni::JavaVM::from_raw(jvm), jni_activity) + }; + // Note: At this point we can assume jni::JavaVM::singleton is initialized + + // Since this is a newly spawned thread then the JVM hasn't been attached to the + // thread yet. + // + // For compatibility we attach before calling the applications main function to + // allow it to assume the thread is attached before making JNI calls. + jvm.attach_current_thread_with_config( + || AttachConfig::new().name("android_main"), + Some(16), + |env| -> jni::errors::Result<()> { + // 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)? }; + + if let Err(err) = init_android_main_thread(&jvm, &jni_activity) { + eprintln!( + "Failed to name Java thread and set thread context class loader: {err}" + ); + } + + let app = AndroidApp::new(rust_glue.clone(), jvm.clone()); + + rust_glue.notify_main_thread_running(); + + unsafe { + // We want to specifically catch any panic from the application's android_main + // so we can finish + destroy the Activity gracefully via the JVM + catch_unwind(|| { + // XXX: If we were in control of the Java Activity subclass then + // we could potentially run the android_main function via a Java native method + // springboard (e.g. call an Activity subclass method that calls a jni native + // method that then just calls android_main()) that would make sure there was + // a Java frame at the base of our call stack which would then be recognised + // when calling FindClass to lookup a suitable classLoader, instead of + // defaulting to the system loader. Without this then it's difficult for native + // code to look up non-standard Java classes. + android_main(app); + }) + .unwrap_or_else(log_panic); + + // Let JVM know that our Activity can be destroyed before detaching from the JVM + // + // "Note that this method can be called from any thread; it will send a message + // to the main thread of the process where the Java finish call will take place" + ndk_sys::ANativeActivity_finish(activity); + + ndk_context::release_android_context(); + } + + rust_glue.notify_main_thread_stopped_running(); + + Ok(()) + }, + ) + .expect("Failed to attach thread to JVM"); + }) +} diff --git a/android-activity/src/native_activity/mod.rs b/android-activity/src/native_activity/mod.rs index 4fa295d..27a25d5 100644 --- a/android-activity/src/native_activity/mod.rs +++ b/android-activity/src/native_activity/mod.rs @@ -6,15 +6,15 @@ use std::ptr::NonNull; use std::sync::{Arc, Mutex, RwLock, Weak}; use std::time::Duration; +use jni::JavaVM; use libc::c_void; use log::{error, trace}; use ndk::input_queue::InputQueue; use ndk::{asset::AssetManager, native_window::NativeWindow}; use crate::error::InternalResult; -use crate::input::{Axis, KeyCharacterMap, KeyCharacterMapBinding}; +use crate::input::{device_key_character_map, Axis, KeyCharacterMap}; use crate::input::{TextInputState, TextSpan}; -use crate::jni_utils::{self, CloneJavaVM}; use crate::{ util, AndroidApp, ConfigurationRef, InputStatus, MainEvent, PollEvent, Rect, WindowManagerFlags, }; @@ -84,50 +84,47 @@ impl AndroidAppWaker { } impl AndroidApp { - pub(crate) fn new(native_activity: NativeActivityGlue, jvm: CloneJavaVM) -> Self { - let mut env = jvm.get_env().unwrap(); // We attach to the thread before creating the AndroidApp + pub(crate) fn new(native_activity: NativeActivityGlue, jvm: JavaVM) -> Self { + jvm.with_local_frame(10, |env| -> jni::errors::Result<_> { + if let Err(err) = crate::input::jni_init(env) { + panic!("Failed to init JNI bindings: {err:?}"); + }; - let key_map_binding = match KeyCharacterMapBinding::new(&mut env) { - Ok(b) => b, - Err(err) => { - panic!("Failed to create KeyCharacterMap JNI bindings: {err:?}"); + let app = Self { + inner: Arc::new(RwLock::new(AndroidAppInner { + jvm: jvm.clone(), + native_activity, + looper: Looper { + ptr: ptr::null_mut(), + }, + key_maps: Mutex::new(HashMap::new()), + input_receiver: Mutex::new(None), + })), + }; + + { + let mut guard = app.inner.write().unwrap(); + + let main_fd = guard.native_activity.cmd_read_fd(); + unsafe { + guard.looper.ptr = ndk_sys::ALooper_prepare( + ndk_sys::ALOOPER_PREPARE_ALLOW_NON_CALLBACKS as libc::c_int, + ); + ndk_sys::ALooper_addFd( + guard.looper.ptr, + main_fd, + LOOPER_ID_MAIN, + ndk_sys::ALOOPER_EVENT_INPUT as libc::c_int, + None, + //&mut guard.cmd_poll_source as *mut _ as *mut _); + ptr::null_mut(), + ); + } } - }; - let app = Self { - inner: Arc::new(RwLock::new(AndroidAppInner { - jvm, - native_activity, - looper: Looper { - ptr: ptr::null_mut(), - }, - key_map_binding: Arc::new(key_map_binding), - key_maps: Mutex::new(HashMap::new()), - input_receiver: Mutex::new(None), - })), - }; - - { - let mut guard = app.inner.write().unwrap(); - - let main_fd = guard.native_activity.cmd_read_fd(); - unsafe { - guard.looper.ptr = ndk_sys::ALooper_prepare( - ndk_sys::ALOOPER_PREPARE_ALLOW_NON_CALLBACKS as libc::c_int, - ); - ndk_sys::ALooper_addFd( - guard.looper.ptr, - main_fd, - LOOPER_ID_MAIN, - ndk_sys::ALOOPER_EVENT_INPUT as libc::c_int, - None, - //&mut guard.cmd_poll_source as *mut _ as *mut _); - ptr::null_mut(), - ); - } - } - - app + Ok(app) + }) + .expect("Failed to create AndroidApp instance") } } @@ -140,14 +137,11 @@ unsafe impl Sync for Looper {} #[derive(Debug)] pub(crate) struct AndroidAppInner { - pub(crate) jvm: CloneJavaVM, + pub(crate) jvm: JavaVM, pub(crate) native_activity: NativeActivityGlue, looper: Looper, - /// Shared JNI bindings for the `KeyCharacterMap` class - key_map_binding: Arc, - /// A table of `KeyCharacterMap`s per `InputDevice` ID /// these are used to be able to map key presses to unicode /// characters @@ -396,11 +390,7 @@ impl AndroidAppInner { let key_map = match guard.entry(device_id) { std::collections::hash_map::Entry::Occupied(occupied) => occupied.get().clone(), std::collections::hash_map::Entry::Vacant(vacant) => { - let character_map = jni_utils::device_key_character_map( - self.jvm.clone(), - self.key_map_binding.clone(), - device_id, - )?; + let character_map = device_key_character_map(self.jvm.clone(), device_id)?; vacant.insert(character_map.clone()); character_map } diff --git a/android-activity/src/util.rs b/android-activity/src/util.rs index b7e774d..0c3b5da 100644 --- a/android-activity/src/util.rs +++ b/android-activity/src/util.rs @@ -1,3 +1,8 @@ +use jni::{ + jni_str, + objects::{JObject, JString, JThread}, + vm::JavaVM, +}; use log::{error, Level}; use std::{ ffi::{CStr, CString}, @@ -111,3 +116,29 @@ pub(crate) fn abort_on_panic(f: impl FnOnce() -> R) -> R { std::process::abort(); }) } + +/// Name the Java Thread + native thread "android_main" and set the Java Thread context class loader +/// so that jni code can more-easily find non-system Java classes. +pub(crate) fn init_android_main_thread( + vm: &JavaVM, + jni_activity: &JObject, +) -> jni::errors::Result<()> { + vm.with_local_frame(10, |env| -> jni::errors::Result<()> { + let activity_class = env.get_object_class(jni_activity)?; + let class_loader = activity_class.get_class_loader(env)?; + + let thread = JThread::current_thread(env)?; + thread.set_context_class_loader(env, &class_loader)?; + let thread_name = JString::from_jni_str(env, jni_str!("android_main"))?; + thread.set_name(env, &thread_name)?; + + // Also name native thread - this needs to happen here after attaching to a JVM thread, + // since that changes the thread name to something like "Thread-2". + unsafe { + let thread_name = std::ffi::CStr::from_bytes_with_nul(b"android_main\0").unwrap(); + let _ = libc::pthread_setname_np(libc::pthread_self(), thread_name.as_ptr()); + } + + Ok(()) + }) +}