From 3843a7cfaa04c3981c1e655db6bfa6e603924ebd Mon Sep 17 00:00:00 2001 From: Robert Bragg Date: Thu, 27 Apr 2023 22:00:43 +0100 Subject: [PATCH] Call Activity.finish() when android_main returns Calling Activity.finish() is what ensures the Activity will get gracefully destroyed, including calling the Activity's onDestroy method. Fixes: #67 --- android-activity/CHANGELOG.md | 2 + android-activity/src/game_activity/mod.rs | 73 ++++++++++++-------- android-activity/src/native_activity/glue.rs | 37 +++++++--- android-activity/src/util.rs | 40 +++++------ 4 files changed, 95 insertions(+), 57 deletions(-) diff --git a/android-activity/CHANGELOG.md b/android-activity/CHANGELOG.md index 6ddb0e12..6124643f 100644 --- a/android-activity/CHANGELOG.md +++ b/android-activity/CHANGELOG.md @@ -3,6 +3,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Changed +- The `Activity.finish()` method is now called when `android_main` returns so the `Activity` will be destroyed ([#67](https://github.com/rust-mobile/android-activity/issues/67)) ## [0.4.1] - 2022-02-16 ### Added diff --git a/android-activity/src/game_activity/mod.rs b/android-activity/src/game_activity/mod.rs index 82bc3e31..d43dc6ec 100644 --- a/android-activity/src/game_activity/mod.rs +++ b/android-activity/src/game_activity/mod.rs @@ -6,6 +6,7 @@ use std::io::{BufRead, BufReader}; use std::marker::PhantomData; use std::ops::Deref; use std::os::unix::prelude::*; +use std::panic::catch_unwind; use std::ptr::NonNull; use std::sync::{Arc, RwLock}; use std::time::Duration; @@ -23,7 +24,7 @@ use ndk::asset::AssetManager; use ndk::configuration::Configuration; use ndk::native_window::NativeWindow; -use crate::util::{abort_on_panic, android_log}; +use crate::util::{abort_on_panic, android_log, log_panic}; use crate::{ util, AndroidApp, ConfigurationRef, InputStatus, MainEvent, PollEvent, Rect, WindowManagerFlags, }; @@ -603,7 +604,8 @@ extern "Rust" { // `app_main` function. This is run on a dedicated thread spawned // by android_native_app_glue. #[no_mangle] -pub unsafe extern "C" fn _rust_glue_entry(app: *mut ffi::android_app) { +#[allow(unused_unsafe)] // Otherwise rust 1.64 moans about using unsafe{} in unsafe functions +pub unsafe extern "C" fn _rust_glue_entry(native_app: *mut ffi::android_app) { abort_on_panic(|| { // Maybe make this stdout/stderr redirection an optional / opt-in feature?... let mut logpipe: [RawFd; 2] = Default::default(); @@ -627,34 +629,51 @@ pub unsafe extern "C" fn _rust_glue_entry(app: *mut ffi::android_app) { } }); - let jvm: *mut JavaVM = (*(*app).activity).vm; - let activity: jobject = (*(*app).activity).javaGameActivity; - ndk_context::initialize_android_context(jvm.cast(), activity.cast()); + let jvm = unsafe { + let jvm = (*(*native_app).activity).vm; + let activity: jobject = (*(*native_app).activity).javaGameActivity; + ndk_context::initialize_android_context(jvm.cast(), activity.cast()); + + // 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 + let mut jenv_out: *mut core::ffi::c_void = std::ptr::null_mut(); + if let Some(attach_current_thread) = (*(*jvm)).AttachCurrentThread { + attach_current_thread(jvm, &mut jenv_out, std::ptr::null_mut()); + } - let app = AndroidApp::from_ptr(NonNull::new(app).unwrap()); + jvm + }; - // 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 - let mut jenv_out: *mut core::ffi::c_void = std::ptr::null_mut(); - if let Some(attach_current_thread) = (*(*jvm)).AttachCurrentThread { - attach_current_thread(jvm, &mut jenv_out, std::ptr::null_mut()); - } + unsafe { + let app = AndroidApp::from_ptr(NonNull::new(native_app).unwrap()); + + // 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(|panic| log_panic(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); - // 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); - - if let Some(detach_current_thread) = (*(*jvm)).DetachCurrentThread { - detach_current_thread(jvm); - } + if let Some(detach_current_thread) = (*(*jvm)).DetachCurrentThread { + detach_current_thread(jvm); + } - ndk_context::release_android_context(); + ndk_context::release_android_context(); + } }) } diff --git a/android-activity/src/native_activity/glue.rs b/android-activity/src/native_activity/glue.rs index 6af66af8..64df1084 100644 --- a/android-activity/src/native_activity/glue.rs +++ b/android-activity/src/native_activity/glue.rs @@ -8,6 +8,7 @@ use std::{ io::{BufRead, BufReader}, ops::Deref, os::unix::prelude::{FromRawFd, RawFd}, + panic::catch_unwind, ptr::{self, NonNull}, sync::{Arc, Condvar, Mutex, Weak}, }; @@ -15,7 +16,11 @@ use std::{ use log::Level; use ndk::{configuration::Configuration, input_queue::InputQueue, native_window::NativeWindow}; -use crate::{util::abort_on_panic, util::android_log, ConfigurationRef}; +use crate::{ + util::android_log, + util::{abort_on_panic, log_panic}, + ConfigurationRef, +}; use super::{AndroidApp, Rect}; @@ -803,6 +808,7 @@ unsafe extern "C" fn on_content_rect_changed( /// This is the native entrypoint for our cdylib library that `ANativeActivity` will look for via `dlsym` #[no_mangle] +#[allow(unused_unsafe)] // Otherwise rust 1.64 moans about using unsafe{} in unsafe functions extern "C" fn ANativeActivity_onCreate( activity: *mut ndk_sys::ANativeActivity, saved_state: *const libc::c_void, @@ -874,15 +880,26 @@ extern "C" fn ANativeActivity_onCreate( rust_glue.notify_main_thread_running(); unsafe { - // 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); + // 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(|panic| log_panic(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); if let Some(detach_current_thread) = (*(*jvm)).DetachCurrentThread { detach_current_thread(jvm); diff --git a/android-activity/src/util.rs b/android-activity/src/util.rs index c1153287..6e047584 100644 --- a/android-activity/src/util.rs +++ b/android-activity/src/util.rs @@ -31,6 +31,25 @@ pub(crate) fn android_log(level: Level, tag: &CStr, msg: &CStr) { } } +pub(crate) fn log_panic(panic: Box) { + let rust_panic = unsafe { CStr::from_bytes_with_nul_unchecked(b"RustPanic\0") }; + + if let Some(panic) = panic.downcast_ref::() { + if let Ok(msg) = CString::new(panic.clone()) { + android_log(Level::Error, rust_panic, &msg); + } + } else if let Ok(panic) = panic.downcast::<&str>() { + if let Ok(msg) = CString::new(*panic) { + android_log(Level::Error, rust_panic, &msg); + } + } else { + let unknown_panic = unsafe { CStr::from_bytes_with_nul_unchecked(b"UnknownPanic\0") }; + android_log(Level::Error, unknown_panic, unsafe { + CStr::from_bytes_with_nul_unchecked(b"\0") + }); + } +} + /// Run a closure and abort the program if it panics. /// /// This is generally used to ensure Rust callbacks won't unwind past the JNI boundary, which leads @@ -45,26 +64,7 @@ pub(crate) fn abort_on_panic(f: impl FnOnce() -> R) -> R { // // Just in case our attempt to log a panic could itself cause a panic we use a // second catch_unwind here. - let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { - // Try logging the panic, but abort if that fails. - let rust_panic = unsafe { CStr::from_bytes_with_nul_unchecked(b"RustPanic\0") }; - - if let Some(panic) = panic.downcast_ref::() { - if let Ok(msg) = CString::new(panic.clone()) { - android_log(Level::Error, rust_panic, &msg); - } - } else if let Ok(panic) = panic.downcast::<&str>() { - if let Ok(msg) = CString::new(*panic) { - android_log(Level::Error, rust_panic, &msg); - } - } else { - let unknown_panic = - unsafe { CStr::from_bytes_with_nul_unchecked(b"UnknownPanic\0") }; - android_log(Level::Error, unknown_panic, unsafe { - CStr::from_bytes_with_nul_unchecked(b"\0") - }); - } - })); + let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| log_panic(panic))); std::process::abort(); }) }