gxm/app: fix shutdown deadlocks caused by scegxm/clean up session controller (#3950)

* gxm: track sync objects and invalidate on shutdown

* app/gxm: shutdown cleanup

vita3k/android/jni/main_android.cpp

@@ -386,9 +386,6 @@ SDLMAIN_DECLSPEC int SDL_main(int argc, char *argv[]) {
                 if (gl_context)
                     SDL_GL_MakeCurrent(window, nullptr); },
             .after_render_thread_start = {},
-            .before_renderer_cleanup = [window, gl_context]() {
-                if (gl_context)
-                    SDL_GL_MakeCurrent(window, gl_context); },
             .destroy_render_context = [&gl_context]() {
                 if (!gl_context)
                     return;

vita3k/app/include/app/functions.h

@@ -68,7 +68,8 @@ enum class AppRunType {
 
 void init_paths(Root &root_paths);
 bool init(EmuEnvState &state, Config &cfg, const Root &root_paths);
-void deinit(EmuEnvState &state);
+void shutdown_app_runtime(EmuEnvState &state);
+void reset_app_state(EmuEnvState &state);
 bool late_init(EmuEnvState &state);
 void apply_renderer_config(EmuEnvState &emuenv);
 void apply_runtime_settings(EmuEnvState &emuenv);

vita3k/app/include/app/session_controller.h

@@ -51,7 +51,6 @@ struct AppSessionPlatform {
     renderer::WindowCallbacks window_callbacks;
     std::function<void()> before_render_thread_start;
     std::function<void()> after_render_thread_start;
-    std::function<void()> before_renderer_cleanup;
     std::function<void()> destroy_render_context;
 };
 

vita3k/app/src/app_init.cpp

@@ -429,16 +429,10 @@ bool init(EmuEnvState &state, Config &cfg, const Root &root_paths) {
     return true;
 }
 
-void deinit(EmuEnvState &state) {
+void shutdown_app_runtime(EmuEnvState &state) {
     state.audio.stop_all_ports();
 
-    state.display.abort = true;
-    state.renderer->notification_ready.notify_all();
-
-    state.gxm.display_queue.abort();
-
-    state.renderer->render_abort = true;
-    state.renderer->command_finish_one.notify_all();
+    gxm::shutdown(state);
 
     state.net.abort_all();
     state.http.shutdown_connections();
@@ -494,12 +488,14 @@ void deinit(EmuEnvState &state) {
 
     state.kernel.deinit(state.mem);
 
-    app::reset_perf_metrics(state);
-
     state.renderer->cleanup();
     state.renderer.reset();
 
     deinit_mem(state.mem);
+}
+
+void reset_app_state(EmuEnvState &state) {
+    app::reset_perf_metrics(state);
 
     state.app_path.clear();
     state.current_app_title.clear();
@@ -511,6 +507,7 @@ void deinit(EmuEnvState &state) {
     state.clear_app_launch_request();
 
     state.ctrl.reset_runtime();
+    set_current_config(state, "");
 
 #if USE_DISCORD
     if (state.cfg.discord_rich_presence)

vita3k/app/src/session_controller.cpp

@@ -21,6 +21,7 @@
 #include <audio/state.h>
 #include <ctrl/state.h>
 #include <display/state.h>
+#include <gxm/functions.h>
 #include <gxm/state.h>
 #include <interface.h>
 #include <io/state.h>
@@ -184,33 +185,17 @@ void AppSessionController::stop(const AppSessionStopReason reason) {
         set_phase(AppSessionPhase::Stopping);
     }
 
-    const bool has_render_context = static_cast<bool>(active_platform.before_renderer_cleanup)
-        || static_cast<bool>(active_platform.destroy_render_context);
+    const bool has_render_context = static_cast<bool>(active_platform.destroy_render_context);
     const bool needs_renderer_cleanup = renderer_was_initialized || has_render_context;
 
     if (runtime_was_initialized) {
-        if (app_started && emuenv.renderer) {
-            emuenv.display.abort = true;
-            emuenv.renderer->notification_ready.notify_all();
-            emuenv.gxm.display_queue.abort();
-            emuenv.renderer->render_abort = true;
-            emuenv.renderer->command_finish_one.notify_all();
-            renderer::stop_render_thread(*emuenv.renderer);
-        }
-
-        if (needs_renderer_cleanup && active_platform.before_renderer_cleanup)
-            active_platform.before_renderer_cleanup();
-
         if (app_started && reason != AppSessionStopReason::LaunchFailure)
             update_app_time_used(emuenv, emuenv.io.app_path);
 
-        deinit(emuenv);
+        shutdown_app_runtime(emuenv);
+        reset_app_state(emuenv);
         destroy(emuenv);
-        set_current_config(emuenv, "");
     } else if (renderer_was_initialized) {
-        if (needs_renderer_cleanup && active_platform.before_renderer_cleanup)
-            active_platform.before_renderer_cleanup();
-
         if (emuenv.renderer) {
             emuenv.renderer->cleanup();
             emuenv.renderer.reset();

vita3k/gui-qt/src/main_window.cpp

@@ -1057,9 +1057,6 @@ std::optional<AppLaunchRequest> MainWindow::boot_game_once(const AppLaunchReques
         platform.after_render_thread_start = [win = m_game_window]() {
             win->complete_gl_migration();
         };
-        platform.before_renderer_cleanup = [win = m_game_window]() {
-            win->make_current();
-        };
         platform.destroy_render_context = [win = m_game_window]() {
             win->destroy_gl_context();
         };

vita3k/gxm/include/gxm/functions.h

@@ -24,6 +24,7 @@
 #include <string>
 
 struct EmuEnvState;
+struct GxmState;
 
 namespace gxm {
 // Color.
@@ -56,6 +57,8 @@ uint32_t get_bits_per_pixel(SceGxmTransferFormat Format);
 
 void destroy_all_contexts(EmuEnvState &emuenv, bool force_backend_destroy);
 void destroy_all_render_targets(EmuEnvState &emuenv, bool force_backend_destroy);
+void shutdown(EmuEnvState &emuenv);
+void invalidate_sync_objects(GxmState &gxm);
 } // namespace gxm
 
 namespace gxp {

vita3k/gxm/include/gxm/state.h

@@ -25,6 +25,7 @@
 #include <mutex>
 #include <thread>
 #include <unordered_map>
+#include <unordered_set>
 
 struct EmuEnvState;
 struct SceGxmContext;
@@ -67,6 +68,9 @@ struct GxmState {
 
     Ptr<uint32_t> notification_region;
 
+    std::mutex sync_objects_mutex;
+    std::unordered_set<SceGxmSyncObject *> sync_objects;
+
     std::map<Address, MemoryMapInfo> memory_mapped_regions;
     std::mutex callback_lock;
     Address immediate_context = 0;

vita3k/modules/SceGxm/SceGxm.cpp

@@ -910,15 +910,12 @@ static void display_entry_thread(EmuEnvState &emuenv) {
         SceGxmSyncObject *new_sync = display_callback->new_sync.get(emuenv.mem);
 
         // sceGxmDisplayQueueAddEntry waits for both buffers to complete
-        // bit ugly but needed to avoid deadlocking during deinit
-        while (!renderer::wishlist(old_sync, display_callback->old_sync_timestamp, 100000)) {
-            if (emuenv.display.abort.load())
-                return;
+        if (renderer::wishlist(old_sync, display_callback->old_sync_timestamp) == renderer::SyncWaitResult::Shutdown) {
+            return;
         }
         if (old_sync != new_sync) {
-            while (!renderer::wishlist(new_sync, display_callback->new_sync_timestamp, 100000)) {
-                if (emuenv.display.abort.load())
-                    return;
+            if (renderer::wishlist(new_sync, display_callback->new_sync_timestamp) == renderer::SyncWaitResult::Shutdown) {
+                return;
             }
         }
 
@@ -1321,6 +1318,29 @@ void destroy_all_contexts(EmuEnvState &emuenv, const bool force_backend_destroy)
     }
 }
 
+void invalidate_sync_objects(GxmState &gxm) {
+    std::lock_guard<std::mutex> lock(gxm.sync_objects_mutex);
+    for (SceGxmSyncObject *sync_object : gxm.sync_objects) {
+        {
+            std::lock_guard<std::mutex> sync_lock(sync_object->lock);
+            sync_object->being_deleted = true;
+        }
+        sync_object->cond.notify_all();
+    }
+}
+
+void shutdown(EmuEnvState &emuenv) {
+    emuenv.display.abort = true;
+    emuenv.renderer->notification_ready.notify_all();
+    emuenv.gxm.display_queue.abort();
+    emuenv.renderer->render_abort = true;
+    invalidate_sync_objects(emuenv.gxm);
+    emuenv.renderer->command_finish_one.notify_all();
+
+    // wait for any deferred GXM callback to finish before continuing shutdown
+    const std::lock_guard<std::mutex> callback_guard(emuenv.gxm.callback_lock);
+}
+
 } // namespace gxm
 
 struct SceGxmRenderTarget {
@@ -4831,6 +4851,10 @@ EXPORT(int, sceGxmSyncObjectCreate, Ptr<SceGxmSyncObject> *syncObject) {
     }
 
     renderer::create(syncObject->get(emuenv.mem), *emuenv.renderer);
+    {
+        std::lock_guard<std::mutex> lock(emuenv.gxm.sync_objects_mutex);
+        emuenv.gxm.sync_objects.emplace(syncObject->get(emuenv.mem));
+    }
 
     return 0;
 }
@@ -4840,6 +4864,10 @@ EXPORT(int, sceGxmSyncObjectDestroy, Ptr<SceGxmSyncObject> syncObject) {
     if (!syncObject)
         return RET_ERROR(SCE_GXM_ERROR_INVALID_POINTER);
 
+    {
+        std::lock_guard<std::mutex> lock(emuenv.gxm.sync_objects_mutex);
+        emuenv.gxm.sync_objects.erase(syncObject.get(emuenv.mem));
+    }
     renderer::destroy(syncObject.get(emuenv.mem), *emuenv.renderer);
     free(emuenv.mem, syncObject);
 

vita3k/renderer/include/renderer/functions.h

@@ -43,12 +43,18 @@ void create(SceGxmSyncObject *sync, State &state);
 void destroy(SceGxmSyncObject *sync, State &state);
 void finish(State &state, Context *context);
 
+enum class SyncWaitResult {
+    Ready,
+    TimedOut,
+    Shutdown
+};
+
 /**
  * \brief Wait for all subjects to be done with the given sync object.
  *
- * Return true if the wait didn't timeout
+ * Return the reason the wait completed.
  */
-bool wishlist(SceGxmSyncObject *sync_object, const uint32_t timestamp, const int32_t timeout_micros = -1);
+SyncWaitResult wishlist(SceGxmSyncObject *sync_object, const uint32_t timestamp, const int32_t timeout_micros = -1);
 
 /**
  * \brief Set list of subject with sync object to done.

vita3k/renderer/include/renderer/gxm_types.h

@@ -178,6 +178,9 @@ struct SceGxmSyncObject {
     // last signal operation done, given using the global timestamp
     uint32_t last_operation_global = 0;
 
+    // Set during emulated app shutdown to release any waiters blocked on this sync object.
+    bool being_deleted = false;
+
     std::mutex lock;
     std::condition_variable cond;
 };

vita3k/renderer/src/batch.cpp

@@ -65,7 +65,7 @@ bool is_cmd_ready(MemState &mem, CommandList &command_list) {
     return sync->timestamp_current >= timestamp;
 }
 
-static bool wait_cmd(MemState &mem, CommandList &command_list) {
+static renderer::SyncWaitResult wait_cmd(MemState &mem, CommandList &command_list) {
     // we assume here that the cmd starts with a WaitSyncObject
 
     SceGxmSyncObject *sync = reinterpret_cast<Ptr<SceGxmSyncObject> *>(&command_list.first->data[0])->get(mem);
@@ -132,6 +132,9 @@ void process_batches(renderer::State &state, const FeatureState &features, MemSt
     auto max_time = duration_cast<std::chrono::milliseconds>(std::chrono::system_clock::now().time_since_epoch()).count() + max_wait_ms;
 
     while (!state.should_display) {
+        if (state.render_abort.load(std::memory_order_relaxed))
+            return;
+
         // overlay requested an async present
         if (state.async_flip_requested.load(std::memory_order_relaxed))
             return;
@@ -148,7 +151,13 @@ void process_batches(renderer::State &state, const FeatureState &features, MemSt
             if (state.current_backend == Backend::OpenGL && config.current_config.v_sync)
                 return;
 
-            if (!cmd_list || !wait_cmd(mem, *cmd_list)) {
+            renderer::SyncWaitResult wait_result = renderer::SyncWaitResult::TimedOut;
+            if (cmd_list)
+                wait_result = wait_cmd(mem, *cmd_list);
+            if (!cmd_list || wait_result != renderer::SyncWaitResult::Ready) {
+                if (wait_result == renderer::SyncWaitResult::Shutdown)
+                    return;
+
                 if (state.async_flip_requested.load(std::memory_order_relaxed))
                     return;
 

vita3k/renderer/src/creation.cpp

@@ -246,6 +246,7 @@ void create(SceGxmSyncObject *sync, State &state) {
     sync->last_display = 0;
     sync->timestamp_current = 0;
     sync->timestamp_ahead = 0;
+    sync->being_deleted = false;
 }
 
 void destroy(SceGxmSyncObject *sync, State &state) {

vita3k/renderer/src/sync.cpp

@@ -149,20 +149,22 @@ int wait_for_status(State &state, int *status, int signal, bool wake_on_equal) {
     return *status;
 }
 
-bool wishlist(SceGxmSyncObject *sync_object, const uint32_t timestamp, const int32_t timeout_micros) {
+SyncWaitResult wishlist(SceGxmSyncObject *sync_object, const uint32_t timestamp, const int32_t timeout_micros) {
     std::unique_lock<std::mutex> lock(sync_object->lock);
     if (sync_object->timestamp_current < timestamp) {
-        const auto &pred = [&]() { return sync_object->timestamp_current >= timestamp; };
+        const auto &pred = [&]() {
+            return sync_object->being_deleted || sync_object->timestamp_current >= timestamp;
+        };
 
         if (timeout_micros == -1) {
             sync_object->cond.wait(lock, pred);
-            return true;
-        } else {
-            return sync_object->cond.wait_for(lock, std::chrono::microseconds(timeout_micros), pred);
+        } else if (!sync_object->cond.wait_for(lock, std::chrono::microseconds(timeout_micros), pred)) {
+            return SyncWaitResult::TimedOut;
         }
-    } else {
-        return true;
     }
+    if (sync_object->being_deleted)
+        return SyncWaitResult::Shutdown;
+    return SyncWaitResult::Ready;
 }
 
 void subject_done(SceGxmSyncObject *sync_object, const uint32_t timestamp) {