From 949386ea4ee5ecdb5ca50da26cb505fa9ab6fd8c Mon Sep 17 00:00:00 2001 From: Robert Bragg Date: Tue, 11 Oct 2022 17:13:50 +0100 Subject: [PATCH] native-activity: fix Weak ref from_raw + upgrade + into_raw handling for callbacks This adds a common `try_with_waitable_activity_ref` utility that handles upgrading the Weak reference associated with `ANativeActivity` whenever we get an activity callback. Previously we weren't converting the Weak reference back to a pointer before returning from the callback which would result in us dropping the Weak reference after the first callback. --- android-activity/src/native_activity/glue.rs | 90 ++++++++++---------- 1 file changed, 47 insertions(+), 43 deletions(-) diff --git a/android-activity/src/native_activity/glue.rs b/android-activity/src/native_activity/glue.rs index ecfdc5a..2be3216 100644 --- a/android-activity/src/native_activity/glue.rs +++ b/android-activity/src/native_activity/glue.rs @@ -650,28 +650,40 @@ fn android_log(level: Level, tag: &CStr, msg: &CStr) { } } +unsafe fn try_with_waitable_activity_ref( + activity: *mut ndk_sys::ANativeActivity, + closure: impl FnOnce(Arc), +) { + assert!(!(*activity).instance.is_null()); + let weak_ptr: *const WaitableNativeActivityState = (*activity).instance.cast(); + let weak_ref = Weak::from_raw(weak_ptr); + if let Some(waitable_activity) = weak_ref.upgrade() { + closure(waitable_activity); + } else { + log::error!("Ignoring spurious JVM callback after last activity reference was dropped!") + } + let _ = weak_ref.into_raw(); +} + unsafe extern "C" fn on_destroy(activity: *mut ndk_sys::ANativeActivity) { log::debug!("Destroy: {:p}\n", activity); - let weak_ptr: *const WaitableNativeActivityState = (*activity).instance.cast(); - if let Some(waitable_activity) = Weak::from_raw(weak_ptr).upgrade() { + try_with_waitable_activity_ref(activity, |waitable_activity| { waitable_activity.notify_destroyed() - } + }); } unsafe extern "C" fn on_start(activity: *mut ndk_sys::ANativeActivity) { log::debug!("Start: {:p}\n", activity); - let weak_ptr: *const WaitableNativeActivityState = (*activity).instance.cast(); - if let Some(waitable_activity) = Weak::from_raw(weak_ptr).upgrade() { + try_with_waitable_activity_ref(activity, |waitable_activity| { waitable_activity.set_activity_state(State::Start); - } + }); } unsafe extern "C" fn on_resume(activity: *mut ndk_sys::ANativeActivity) { log::debug!("Resume: {:p}\n", activity); - let weak_ptr: *const WaitableNativeActivityState = (*activity).instance.cast(); - if let Some(waitable_activity) = Weak::from_raw(weak_ptr).upgrade() { + try_with_waitable_activity_ref(activity, |waitable_activity| { waitable_activity.set_activity_state(State::Resume); - } + }); } unsafe extern "C" fn on_save_instance_state( @@ -679,47 +691,44 @@ unsafe extern "C" fn on_save_instance_state( out_len: *mut ndk_sys::size_t, ) -> *mut libc::c_void { log::debug!("SaveInstanceState: {:p}\n", activity); - let weak_ptr: *const WaitableNativeActivityState = (*activity).instance.cast(); - if let Some(waitable_activity) = Weak::from_raw(weak_ptr).upgrade() { + *out_len = 0; + let mut ret = ptr::null_mut(); + try_with_waitable_activity_ref(activity, |waitable_activity| { let (state, len) = waitable_activity.request_save_state(); *out_len = len as ndk_sys::size_t; - state - } else { - *out_len = 0; - ptr::null_mut() - } + ret = state + }); + + log::debug!("Saved state = {:p}, len = {}", ret, *out_len); + ret } unsafe extern "C" fn on_pause(activity: *mut ndk_sys::ANativeActivity) { log::debug!("Pause: {:p}\n", activity); - let weak_ptr: *const WaitableNativeActivityState = (*activity).instance.cast(); - if let Some(waitable_activity) = Weak::from_raw(weak_ptr).upgrade() { + try_with_waitable_activity_ref(activity, |waitable_activity| { waitable_activity.set_activity_state(State::Pause); - } + }); } unsafe extern "C" fn on_stop(activity: *mut ndk_sys::ANativeActivity) { log::debug!("Stop: {:p}\n", activity); - let weak_ptr: *const WaitableNativeActivityState = (*activity).instance.cast(); - if let Some(waitable_activity) = Weak::from_raw(weak_ptr).upgrade() { + try_with_waitable_activity_ref(activity, |waitable_activity| { waitable_activity.set_activity_state(State::Stop); - } + }); } unsafe extern "C" fn on_configuration_changed(activity: *mut ndk_sys::ANativeActivity) { log::debug!("ConfigurationChanged: {:p}\n", activity); - let weak_ptr: *const WaitableNativeActivityState = (*activity).instance.cast(); - if let Some(waitable_activity) = Weak::from_raw(weak_ptr).upgrade() { + try_with_waitable_activity_ref(activity, |waitable_activity| { waitable_activity.notify_config_changed(); - } + }); } unsafe extern "C" fn on_low_memory(activity: *mut ndk_sys::ANativeActivity) { log::debug!("LowMemory: {:p}\n", activity); - let weak_ptr: *const WaitableNativeActivityState = (*activity).instance.cast(); - if let Some(waitable_activity) = Weak::from_raw(weak_ptr).upgrade() { + try_with_waitable_activity_ref(activity, |waitable_activity| { waitable_activity.notify_low_memory(); - } + }); } unsafe extern "C" fn on_window_focus_changed( @@ -727,10 +736,9 @@ unsafe extern "C" fn on_window_focus_changed( focused: libc::c_int, ) { log::debug!("WindowFocusChanged: {:p} -- {}\n", activity, focused); - let weak_ptr: *const WaitableNativeActivityState = (*activity).instance.cast(); - if let Some(waitable_activity) = Weak::from_raw(weak_ptr).upgrade() { + try_with_waitable_activity_ref(activity, |waitable_activity| { waitable_activity.notify_focus_changed(focused != 0); - } + }); } unsafe extern "C" fn on_native_window_created( @@ -738,13 +746,12 @@ unsafe extern "C" fn on_native_window_created( window: *mut ndk_sys::ANativeWindow, ) { log::debug!("NativeWindowCreated: {:p} -- {:p}\n", activity, window); - let weak_ptr: *const WaitableNativeActivityState = (*activity).instance.cast(); - if let Some(waitable_activity) = Weak::from_raw(weak_ptr).upgrade() { + try_with_waitable_activity_ref(activity, |waitable_activity| { // It's important that we use ::clone_from_ptr() here because NativeWindow // has a Drop implementation that will unconditionally _release() the native window let window = NativeWindow::clone_from_ptr(NonNull::new_unchecked(window)); waitable_activity.set_window(Some(window)); - } + }); } unsafe extern "C" fn on_native_window_destroyed( @@ -752,10 +759,9 @@ unsafe extern "C" fn on_native_window_destroyed( window: *mut ndk_sys::ANativeWindow, ) { log::debug!("NativeWindowDestroyed: {:p} -- {:p}\n", activity, window); - let weak_ptr: *const WaitableNativeActivityState = (*activity).instance.cast(); - if let Some(waitable_activity) = Weak::from_raw(weak_ptr).upgrade() { + try_with_waitable_activity_ref(activity, |waitable_activity| { waitable_activity.set_window(None); - } + }); } unsafe extern "C" fn on_input_queue_created( @@ -763,10 +769,9 @@ unsafe extern "C" fn on_input_queue_created( queue: *mut ndk_sys::AInputQueue, ) { log::debug!("InputQueueCreated: {:p} -- {:p}\n", activity, queue); - let weak_ptr: *const WaitableNativeActivityState = (*activity).instance.cast(); - if let Some(waitable_activity) = Weak::from_raw(weak_ptr).upgrade() { + try_with_waitable_activity_ref(activity, |waitable_activity| { waitable_activity.set_input(queue); - } + }); } unsafe extern "C" fn on_input_queue_destroyed( @@ -774,10 +779,9 @@ unsafe extern "C" fn on_input_queue_destroyed( queue: *mut ndk_sys::AInputQueue, ) { log::debug!("InputQueueDestroyed: {:p} -- {:p}\n", activity, queue); - let weak_ptr: *const WaitableNativeActivityState = (*activity).instance.cast(); - if let Some(waitable_activity) = Weak::from_raw(weak_ptr).upgrade() { + try_with_waitable_activity_ref(activity, |waitable_activity| { waitable_activity.set_input(ptr::null_mut()); - } + }); } /// This is the native entrypoint for our cdylib library that `ANativeActivity` will look for via `dlsym`