From 3d37cc00415600a6b03789c3d518a78e9b0a3239 Mon Sep 17 00:00:00 2001 From: Robert Bragg Date: Mon, 2 Mar 2026 23:40:41 +0000 Subject: [PATCH] Ensure AndroidAppWaker owns an ALooper reference This ensures we call `ALooper_acquire` before `create_waker()` wraps the Looper pointer with `AndroidAppWaker` and it also ensures that `::clone()` and `::drop()` call `ALooper_acquire()` and `ALooper_release()` respectively. Contrary to what the comment for the `looper` member said previously, it was not safe to assume that the application's looper pointer had a `'static` lifetime. The looper pointer would only be valid up until `android_main` returns, but unlike a traditional `main()` function an `android_main()` runs with respect to an `Activity` lifecycle and not a process lifecycle. It's technically possible for `android_main()` to return (at which point any looper stored in `'static` storage would have previously become an invalid pointer) and then JNI could be used to re-enter Rust and potentially try and dereference that invalid pointer. This adds a shared implementation of `AndroidAppWaker` to `src/waker.rs` instead of having each backend implement `AndroidAppWaker`. Fixes: #226 --- android-activity/src/game_activity/mod.rs | 30 ++--------- android-activity/src/lib.rs | 4 +- android-activity/src/native_activity/mod.rs | 37 ++----------- android-activity/src/waker.rs | 57 +++++++++++++++++++++ 4 files changed, 69 insertions(+), 59 deletions(-) create mode 100644 android-activity/src/waker.rs diff --git a/android-activity/src/game_activity/mod.rs b/android-activity/src/game_activity/mod.rs index 1b589c6..fcfd3d8 100644 --- a/android-activity/src/game_activity/mod.rs +++ b/android-activity/src/game_activity/mod.rs @@ -15,7 +15,7 @@ use log::{error, trace}; use jni::sys::*; -use ndk_sys::{ALooper, ALooper_pollOnce, ALooper_wake}; +use ndk_sys::ALooper_pollOnce; use ndk::asset::AssetManager; use ndk::configuration::Configuration; @@ -27,7 +27,8 @@ use crate::util::{ try_get_path_from_ptr, }; use crate::{ - AndroidApp, ConfigurationRef, InputStatus, MainEvent, PollEvent, Rect, WindowManagerFlags, + AndroidApp, AndroidAppWaker, ConfigurationRef, InputStatus, MainEvent, PollEvent, Rect, + WindowManagerFlags, }; mod ffi; @@ -105,24 +106,6 @@ impl StateLoader<'_> { } } -#[derive(Clone)] -pub struct AndroidAppWaker { - // The looper pointer is owned by the android_app and effectively - // has a 'static lifetime, and the ALooper_wake C API is thread - // safe, so this can be cloned safely and is send + sync safe - looper: NonNull, -} -unsafe impl Send for AndroidAppWaker {} -unsafe impl Sync for AndroidAppWaker {} - -impl AndroidAppWaker { - pub fn wake(&self) { - unsafe { - ALooper_wake(self.looper.as_ptr()); - } - } -} - impl AndroidApp { pub(crate) unsafe fn from_ptr(ptr: NonNull, jvm: jni::JavaVM) -> Self { // We attach to the thread before creating the AndroidApp @@ -619,13 +602,10 @@ impl AndroidAppInner { } pub fn create_waker(&self) -> AndroidAppWaker { + // Safety: we know that the app and looper pointers are valid unsafe { - // From the application's pov we assume the app_ptr and looper pointer - // have static lifetimes and we can safely assume they are never NULL. let app_ptr = self.native_app.as_ptr(); - AndroidAppWaker { - looper: NonNull::new_unchecked((*app_ptr).looper), - } + AndroidAppWaker::new((*app_ptr).looper) } } diff --git a/android-activity/src/lib.rs b/android-activity/src/lib.rs index 3393562..e4ffe76 100644 --- a/android-activity/src/lib.rs +++ b/android-activity/src/lib.rs @@ -187,6 +187,9 @@ mod util; mod jni_utils; +mod waker; +pub use waker::AndroidAppWaker; + /// A rectangle with integer edge coordinates. Used to represent window insets, for example. #[derive(Clone, Debug, Default, Eq, PartialEq)] pub struct Rect { @@ -344,7 +347,6 @@ pub enum InputStatus { } use activity_impl::AndroidAppInner; -pub use activity_impl::AndroidAppWaker; bitflags! { /// Flags for [`AndroidApp::set_window_flags`] diff --git a/android-activity/src/native_activity/mod.rs b/android-activity/src/native_activity/mod.rs index 0a2ef1c..424aceb 100644 --- a/android-activity/src/native_activity/mod.rs +++ b/android-activity/src/native_activity/mod.rs @@ -14,7 +14,8 @@ use ndk::{asset::AssetManager, native_window::NativeWindow}; use crate::error::InternalResult; use crate::{ - util, AndroidApp, ConfigurationRef, InputStatus, MainEvent, PollEvent, Rect, WindowManagerFlags, + util, AndroidApp, AndroidAppWaker, ConfigurationRef, InputStatus, MainEvent, PollEvent, Rect, + WindowManagerFlags, }; pub mod input; @@ -60,31 +61,6 @@ impl StateLoader<'_> { } } -/// A means to wake up the main thread while it is blocked waiting for I/O -#[derive(Clone)] -pub struct AndroidAppWaker { - // The looper pointer is owned by the android_app and effectively - // has a 'static lifetime, and the ALooper_wake C API is thread - // safe, so this can be cloned safely and is send + sync safe - looper: NonNull, -} -unsafe impl Send for AndroidAppWaker {} -unsafe impl Sync for AndroidAppWaker {} - -impl AndroidAppWaker { - /// Interrupts the main thread if it is blocked within [`AndroidApp::poll_events()`] - /// - /// If [`AndroidApp::poll_events()`] is interrupted it will invoke the poll - /// callback with a [PollEvent::Wake][wake_event] event. - /// - /// [wake_event]: crate::PollEvent::Wake - pub fn wake(&self) { - unsafe { - ndk_sys::ALooper_wake(self.looper.as_ptr()); - } - } -} - impl AndroidApp { pub(crate) fn new(native_activity: NativeActivityGlue, jvm: JavaVM) -> Self { jvm.with_local_frame(10, |env| -> jni::errors::Result<_> { @@ -305,13 +281,8 @@ impl AndroidAppInner { } pub fn create_waker(&self) -> AndroidAppWaker { - unsafe { - // From the application's pov we assume the looper pointer has a static - // lifetimes and we can safely assume it is never NULL. - AndroidAppWaker { - looper: NonNull::new_unchecked(self.looper.ptr), - } - } + // Safety: we know that the looper is a valid, non-null pointer + unsafe { AndroidAppWaker::new(self.looper.ptr) } } pub fn config(&self) -> ConfigurationRef { diff --git a/android-activity/src/waker.rs b/android-activity/src/waker.rs new file mode 100644 index 0000000..9997f03 --- /dev/null +++ b/android-activity/src/waker.rs @@ -0,0 +1,57 @@ +use std::ptr::NonNull; + +#[cfg(doc)] +use crate::AndroidApp; + +/// A means to wake up the main thread while it is blocked waiting for I/O +pub struct AndroidAppWaker { + looper: NonNull, +} + +impl Clone for AndroidAppWaker { + fn clone(&self) -> Self { + unsafe { ndk_sys::ALooper_acquire(self.looper.as_ptr()) } + Self { + looper: self.looper, + } + } +} + +impl Drop for AndroidAppWaker { + fn drop(&mut self) { + unsafe { ndk_sys::ALooper_release(self.looper.as_ptr()) } + } +} + +unsafe impl Send for AndroidAppWaker {} +unsafe impl Sync for AndroidAppWaker {} + +impl AndroidAppWaker { + /// Acquire a ref to a looper as a means to be able to wake up the event loop + /// + /// # Safety + /// + /// The `ALooper` pointer must be valid and not null. + pub(crate) unsafe fn new(looper: *mut ndk_sys::ALooper) -> Self { + assert!(!looper.is_null(), "looper pointer must not be null"); + unsafe { + // Give the waker its own reference to the looper + ndk_sys::ALooper_acquire(looper); + AndroidAppWaker { + looper: NonNull::new_unchecked(looper), + } + } + } + + /// Interrupts the main thread if it is blocked within [`AndroidApp::poll_events()`] + /// + /// If [`AndroidApp::poll_events()`] is interrupted it will invoke the poll + /// callback with a [PollEvent::Wake][wake_event] event. + /// + /// [wake_event]: crate::PollEvent::Wake + pub fn wake(&self) { + unsafe { + ndk_sys::ALooper_wake(self.looper.as_ptr()); + } + } +}