From c10a2fb67accd0a4983ede6e021c13518b5ed00c Mon Sep 17 00:00:00 2001 From: Robert Bragg Date: Fri, 28 Jul 2023 22:18:24 +0100 Subject: [PATCH] GameActivity PATH: fix deadlocks in java callbacks after app destroyed This ensures that any java Activity callbacks take into account the possibility that the `android_app` may have already been marked destroyed if `android_main` has returned - and so they mustn't block and wait for a thread that is no longer running. --- android-activity/build.rs | 5 +- .../native_app_glue/android_native_app_glue.c | 63 +++++++++++++++++-- 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/android-activity/build.rs b/android-activity/build.rs index 110859b..305b6ba 100644 --- a/android-activity/build.rs +++ b/android-activity/build.rs @@ -1,10 +1,7 @@ #![allow(dead_code)] fn build_glue_for_game_activity() { - for f in [ - "GameActivity.h", - "GameActivity.cpp", - ] { + for f in ["GameActivity.h", "GameActivity.cpp"] { println!("cargo:rerun-if-changed=game-activity-csrc/game-activity/{f}"); } cc::Build::new() diff --git a/android-activity/game-activity-csrc/game-activity/native_app_glue/android_native_app_glue.c b/android-activity/game-activity-csrc/game-activity/native_app_glue/android_native_app_glue.c index 5fd3eb8..0a52bd0 100644 --- a/android-activity/game-activity-csrc/game-activity/native_app_glue/android_native_app_glue.c +++ b/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); }