Merge pull request #99 from rust-mobile/release-0.4.3

Release 0.4.3

android-activity/CHANGELOG.md

@@ -1,11 +1,18 @@
+<!-- markdownlint-disable MD022 MD024 MD032  -->
+
 # Changelog
 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]
 
+## [0.4.3] - 2022-07-30
+### Fixed
+- Fixed a deadlock in the `native-activity` backend while waiting for the native thread after getting an `onDestroy` callback from Java ([#94](https://github.com/rust-mobile/android-activity/pull/94))
 
-## [0.4.2] - 2022-02-16
+- Fixed numerous deadlocks in the `game-activity` backend with how it would wait for the native thread in various Java callbacks, after the app has returned from `android_main` ([#98](https://github.com/rust-mobile/android-activity/pull/98))
+
+## [0.4.2] - 2022-06-17
 ### 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))
 - The `native-activity` backend now propagates `NativeWindow` redraw/resize and `ContentRectChanged` callbacks to main loop ([#70](https://github.com/rust-mobile/android-activity/pull/70))

android-activity/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "android-activity"
-version = "0.4.2"
+version = "0.4.3"
 edition = "2021"
 keywords = ["android", "ndk"]
 readme = "../README.md"

android-activity/build.rs

@@ -1,6 +1,9 @@
 #![allow(dead_code)]
 
 fn build_glue_for_game_activity() {
+    for f in ["GameActivity.h", "GameActivity.cpp"] {
+        println!("cargo:rerun-if-changed=game-activity-csrc/game-activity/{f}");
+    }
     cc::Build::new()
         .cpp(true)
         .include("game-activity-csrc")
@@ -8,12 +11,20 @@ fn build_glue_for_game_activity() {
         .extra_warnings(false)
         .cpp_link_stdlib("c++_static")
         .compile("libgame_activity.a");
+
+    for f in ["gamecommon.h", "gametextinput.h", "gametextinput.cpp"] {
+        println!("cargo:rerun-if-changed=game-activity-csrc/game-text-input/{f}");
+    }
     cc::Build::new()
         .cpp(true)
         .include("game-activity-csrc")
         .file("game-activity-csrc/game-text-input/gametextinput.cpp")
         .cpp_link_stdlib("c++_static")
         .compile("libgame_text_input.a");
+
+    for f in ["android_native_app_glue.h", "android_native_app_glue.c"] {
+        println!("cargo:rerun-if-changed=game-activity-csrc/native_app_glue/{f}");
+    }
     cc::Build::new()
         .include("game-activity-csrc")
         .include("game-activity-csrc/game-activity/native_app_glue")

android-activity/game-activity-csrc/game-activity/native_app_glue/android_native_app_glue.c

@@ -308,6 +308,15 @@ static void android_app_set_window(struct android_app* android_app,
                                    ANativeWindow* window) {
     LOGV("android_app_set_window called");
     pthread_mutex_lock(&android_app->mutex);
+
+    // NB: we have to consider that the native thread could have already
+    // (gracefully) exit (setting android_app->destroyed) and so we need
+    // to be careful to avoid a deadlock waiting for a thread that's
+    // already exit.
+    if (android_app->destroyed) {
+        pthread_mutex_unlock(&android_app->mutex);
+        return;
+    }
     if (android_app->pendingWindow != NULL) {
         android_app_write_cmd(android_app, APP_CMD_TERM_WINDOW);
     }
@@ -324,15 +333,30 @@ static void android_app_set_window(struct android_app* android_app,
 static void android_app_set_activity_state(struct android_app* android_app,
                                            int8_t cmd) {
     pthread_mutex_lock(&android_app->mutex);
-    android_app_write_cmd(android_app, cmd);
-    while (android_app->activityState != cmd) {
-        pthread_cond_wait(&android_app->cond, &android_app->mutex);
+
+    // NB: we have to consider that the native thread could have already
+    // (gracefully) exit (setting android_app->destroyed) and so we need
+    // to be careful to avoid a deadlock waiting for a thread that's
+    // already exit.
+    if (!android_app->destroyed) {
+        android_app_write_cmd(android_app, cmd);
+        while (android_app->activityState != cmd) {
+            pthread_cond_wait(&android_app->cond, &android_app->mutex);
+        }
     }
     pthread_mutex_unlock(&android_app->mutex);
 }
 
 static void android_app_free(struct android_app* android_app) {
     pthread_mutex_lock(&android_app->mutex);
+
+    // It's possible that onDestroy is called after we have already 'destroyed'
+    // the app (via `android_app_destroy` due to `android_main` returning.
+    //
+    // In this case `->destroyed` will already be set (so we won't deadlock in
+    // the loop below) but we still need to close the messaging fds and finish
+    // freeing the android_app
+
     android_app_write_cmd(android_app, APP_CMD_DESTROY);
     while (!android_app->destroyed) {
         pthread_cond_wait(&android_app->cond, &android_app->mutex);
@@ -372,6 +396,16 @@ static void onSaveInstanceState(GameActivity* activity,
 
     struct android_app* android_app = ToApp(activity);
     pthread_mutex_lock(&android_app->mutex);
+
+    // NB: we have to consider that the native thread could have already
+    // (gracefully) exit (setting android_app->destroyed) and so we need
+    // to be careful to avoid a deadlock waiting for a thread that's
+    // already exit.
+    if (android_app->destroyed) {
+        pthread_mutex_unlock(&android_app->mutex);
+        return;
+    }
+
     android_app->stateSaved = 0;
     android_app_write_cmd(android_app, APP_CMD_SAVE_STATE);
     while (!android_app->stateSaved) {
@@ -481,6 +515,15 @@ static bool onTouchEvent(GameActivity* activity,
     struct android_app* android_app = ToApp(activity);
     pthread_mutex_lock(&android_app->mutex);
 
+    // NB: we have to consider that the native thread could have already
+    // (gracefully) exit (setting android_app->destroyed) and so we need
+    // to be careful to avoid a deadlock waiting for a thread that's
+    // already exit.
+    if (android_app->destroyed) {
+        pthread_mutex_unlock(&android_app->mutex);
+        return false;
+    }
+
     if (android_app->motionEventFilter != NULL &&
         !android_app->motionEventFilter(event)) {
         pthread_mutex_unlock(&android_app->mutex);
@@ -563,6 +606,15 @@ static bool onKey(GameActivity* activity, const GameActivityKeyEvent* event) {
     struct android_app* android_app = ToApp(activity);
     pthread_mutex_lock(&android_app->mutex);
 
+    // NB: we have to consider that the native thread could have already
+    // (gracefully) exit (setting android_app->destroyed) and so we need
+    // to be careful to avoid a deadlock waiting for a thread that's
+    // already exit.
+    if (android_app->destroyed) {
+        pthread_mutex_unlock(&android_app->mutex);
+        return false;
+    }
+
     if (android_app->keyEventFilter != NULL &&
         !android_app->keyEventFilter(event)) {
         pthread_mutex_unlock(&android_app->mutex);
@@ -599,8 +651,9 @@ static void onTextInputEvent(GameActivity* activity,
                              const GameTextInputState* state) {
     struct android_app* android_app = ToApp(activity);
     pthread_mutex_lock(&android_app->mutex);
-
-    android_app->textInputState = 1;
+    if (!android_app->destroyed) {
+        android_app->textInputState = 1;
+    }
     pthread_mutex_unlock(&android_app->mutex);
 }
 

android-activity/src/native_activity/glue.rs

@@ -199,6 +199,18 @@ impl NativeActivityGlue {
     }
 }
 
+/// The status of the native thread that's created to run
+/// `android_main`
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+pub enum NativeThreadState {
+    /// The `android_main` thread hasn't been created yet
+    Init,
+    /// The `android_main` thread has been spawned and started running
+    Running,
+    /// The `android_main` thread has finished
+    Stopped,
+}
+
 #[derive(Debug)]
 pub struct NativeActivityState {
     pub msg_read: libc::c_int,
@@ -210,8 +222,11 @@ pub struct NativeActivityState {
     pub content_rect: ndk_sys::ARect,
     pub activity_state: State,
     pub destroy_requested: bool,
-    pub running: bool,
+    pub thread_state: NativeThreadState,
     pub app_has_saved_state: bool,
+
+    /// Set as soon as the Java main thread notifies us of an
+    /// `onDestroyed` callback.
     pub destroyed: bool,
     pub redraw_needed: bool,
     pub pending_input_queue: *mut ndk_sys::AInputQueue,
@@ -303,8 +318,6 @@ impl Drop for WaitableNativeActivityState {
         unsafe {
             let mut guard = self.mutex.lock().unwrap();
             guard.detach_input_queue_from_looper();
-            guard.destroyed = true;
-            self.cond.notify_one();
         }
     }
 }
@@ -356,7 +369,7 @@ impl WaitableNativeActivityState {
                 content_rect: Rect::empty().into(),
                 activity_state: State::Init,
                 destroy_requested: false,
-                running: false,
+                thread_state: NativeThreadState::Init,
                 app_has_saved_state: false,
                 destroyed: false,
                 redraw_needed: false,
@@ -369,10 +382,11 @@ impl WaitableNativeActivityState {
 
     pub fn notify_destroyed(&self) {
         let mut guard = self.mutex.lock().unwrap();
+        guard.destroyed = true;
 
         unsafe {
             guard.write_cmd(AppCmd::Destroy);
-            while !guard.destroyed {
+            while guard.thread_state != NativeThreadState::Stopped {
                 guard = self.cond.wait(guard).unwrap();
             }
 
@@ -541,7 +555,13 @@ impl WaitableNativeActivityState {
 
     pub fn notify_main_thread_running(&self) {
         let mut guard = self.mutex.lock().unwrap();
-        guard.running = true;
+        guard.thread_state = NativeThreadState::Running;
+        self.cond.notify_one();
+    }
+
+    pub fn notify_main_thread_stopped_running(&self) {
+        let mut guard = self.mutex.lock().unwrap();
+        guard.thread_state = NativeThreadState::Stopped;
         self.cond.notify_one();
     }
 
@@ -907,11 +927,16 @@ extern "C" fn ANativeActivity_onCreate(
 
                 ndk_context::release_android_context();
             }
+
+            rust_glue.notify_main_thread_stopped_running();
         });
 
         // Wait for thread to start.
         let mut guard = jvm_glue.mutex.lock().unwrap();
-        while !guard.running {
+
+        // Don't specifically wait for `Running` just in case `android_main` returns
+        // immediately and the state is set to `Stopped`
+        while guard.thread_state == NativeThreadState::Init {
             guard = jvm_glue.cond.wait(guard).unwrap();
         }
     })