Merge pull request #1876 from malucard/quitting

Fix quitting issue again

Author: nicolasnoble

src/core/DynaRec_aa64/recompiler.cc

@@ -169,7 +169,6 @@ void DynaRecCPU::emitDispatcher() {
     gen.Str(contextPointer,
             MemOperand(sp, -16, PreIndex));  // Save contextPointer register in stack (also align stack pointer)
 
-    gen.Mov(runningPointer, (uintptr_t)PCSX::g_system->runningPtr());  // Move runningPtr to runningPointer register
     gen.Mov(contextPointer, (uintptr_t)this);                          // Load context pointer
 
     // Back up all our allocateable volatile regs
@@ -188,8 +187,12 @@ void DynaRecCPU::emitDispatcher() {
 
     loadThisPointer(arg1.X());  // Poll events
     call(recBranchTestWrapper);
+    gen.Mov(runningPointer, (uintptr_t)PCSX::g_system->runningPtr());  // Move runningPtr to runningPointer register
     gen.Ldrb(w0, MemOperand(runningPointer));  // Check if PCSX::g_system->running is true
     gen.Cbz(w0, &done);                        // If it's not, return
+    gen.Mov(runningPointer, (uintptr_t)PCSX::g_system->quittingPtr());  // Load pointer to "quitting" variable
+    gen.Ldrb(w0, MemOperand(runningPointer));  // Check if PCSX::g_system->quitting is true
+    gen.Cbnz(w0, &done);                       // If it is, return
     emitBlockLookup();                         // Otherwise, look up next block
 
     gen.align();

src/core/DynaRec_x64/recompiler.cc

@@ -186,7 +186,6 @@ void DynaRecCPU::emitDispatcher() {
         gen.push(reg.cvt64());
     }
     gen.mov(qword[contextPointer + HOST_REG_CACHE_OFFSET(0)], runningPointer);  // Backup running pointer
-    gen.mov(runningPointer, (uintptr_t)PCSX::g_system->runningPtr());           // Load pointer to "running" variable
 
     // Allocate shadow stack space on Windows
     if constexpr (isWindows()) {
@@ -202,8 +201,12 @@ void DynaRecCPU::emitDispatcher() {
 
     // Poll events
     emitMemberFunctionCall(&PCSX::R3000Acpu::branchTest, this);
+    gen.mov(runningPointer, (uintptr_t)PCSX::g_system->runningPtr());  // Load pointer to "running" variable
     gen.test(Xbyak::util::byte[runningPointer], 1);  // Check if PCSX::g_system->running is true
     gen.jz(done);                                    // If it's not, return
+    gen.mov(runningPointer, (uintptr_t)PCSX::g_system->quittingPtr());  // Load pointer to "quitting" variable
+    gen.test(Xbyak::util::byte[runningPointer], 1);  // Check if PCSX::g_system->running is true
+    gen.jnz(done);                                   // If it is, return
     emitBlockLookup();                               // Otherwise, look up next block
 
     gen.align(16);

src/core/system.h

@@ -163,8 +163,12 @@ class System {
     // Close mem and plugins
     virtual void close() = 0;
     virtual void purgeAllEvents() = 0;
-    bool running() { return m_running; }
+    bool running() {
+        std::atomic_signal_fence(std::memory_order_relaxed);
+        return m_running && !m_quitting;
+    }
     const bool *runningPtr() { return &m_running; }
+    const bool *quittingPtr() { return &m_quitting; }
     bool quitting() { return m_quitting; }
     int exitCode() { return m_exitCode; }
     bool emergencyExit() { return m_emergencyExit; }
@@ -179,12 +183,10 @@ class System {
         m_eventBus->signal(Events::ExecutionFlow::Run{});
     }
     virtual void testQuit(int code) = 0;
+    // This needs to only mutate variables, as it requires to be signal-safe.
     [[gnu::cold]] void quit(int code = 0) {
         m_quitting = true;
-        pause();
         m_exitCode = code;
-        m_eventBus->signal(Events::Quitting{});
-        purgeAllEvents();
     }
 
     std::shared_ptr<EventBus::EventBus> m_eventBus = std::make_shared<EventBus::EventBus>();
@@ -257,7 +259,17 @@ class System {
     std::map<uint64_t, std::string> m_i18n;
     std::map<std::string, decltype(m_i18n)> m_locales;
     std::string m_currentLocale;
+    // If true, indicates that the emulator is currently capturing the main loop
+    // and actively emulates the PSX hardware. If false, the emulator is paused,
+    // waiting for user input or other events inside the UI. The way the UI
+    // is refreshed is by calling update() periodically, so this boolean affects
+    // the moment when and how update() is called.
     bool m_running = false;
+    // If true, indicates that the emulator is quitting. This can be set by a
+    // number of events, including the user pressing the quit button or the
+    // emulator itself requesting a quit due to testing for instance. This will
+    // cause the two main loop to exit: the inner one being the emulator itself,
+    // and the outer one being the main.cc loop.
     bool m_quitting = false;
     int m_exitCode = 0;
     struct LocaleInfo {

src/main/main.cc

@@ -116,8 +116,6 @@ class SystemImpl final : public PCSX::System {
         // emulator is requesting a shutdown of the emulation
     }
 
-    virtual void purgeAllEvents() final override { uv_run(getLoop(), UV_RUN_DEFAULT); }
-
     virtual void testQuit(int code) final override {
         if (m_args.isTestModeEnabled()) {
             quit(code);
@@ -134,6 +132,8 @@ class SystemImpl final : public PCSX::System {
     const PCSX::Arguments m_args;
 
   public:
+    virtual void purgeAllEvents() final override { uv_run(getLoop(), UV_RUN_DEFAULT); }
+
     void setBinDir(std::filesystem::path path) {
         m_binDir = path;
         m_version.loadFromFile(new PCSX::PosixFile(path / "version.json"));
@@ -166,6 +166,10 @@ struct Cleaner {
     std::function<void()> f;
 };
 
+void handleSignal(int signal) {
+    PCSX::g_system->quit(-1);
+}
+
 int pcsxMain(int argc, char **argv) {
     ZoneScoped;
     // Command line arguments are parsed after this point.
@@ -193,11 +197,10 @@ int pcsxMain(int argc, char **argv) {
     // enabled as much as possible.
     SystemImpl *system = new SystemImpl(args);
     PCSX::g_system = system;
-    static std::atomic_bool scheduledQuit = false;
-    auto sigint = std::signal(SIGINT, [](auto signal) { scheduledQuit = true; });
-    auto sigterm = std::signal(SIGTERM, [](auto signal) { scheduledQuit = true; });
+    auto sigint = std::signal(SIGINT, handleSignal);
+    auto sigterm = std::signal(SIGTERM, handleSignal);
 #ifndef _WIN32
-    signal(SIGPIPE, SIG_IGN);
+    std::signal(SIGPIPE, SIG_IGN);
 #endif
     const auto &logfileArgOpt = args.get<std::string>("logfile");
     const PCSX::u8string logfileArg = MAKEU8(logfileArgOpt.has_value() ? logfileArgOpt->c_str() : "");
@@ -451,15 +454,14 @@ runner.init({
                     emulator->m_cpu->Execute();
                 } else {
                     // The "update" method will be called periodically by the emulator while
-                    // meaning if we want our UI to work, we have to manually call "update"
-                    // when the emulator is paused.
+                    // it's running, meaning if we want our UI to work, we have to manually
+                    // call "update" when the emulator is paused.
                     s_ui->update();
                 }
-                if (scheduledQuit) {
-                    PCSX::g_system->quit(-1);
-                    return exitCode;
-                }
             }
+            system->pause();
+            system->m_eventBus->signal(PCSX::Events::Quitting{});
+            system->purgeAllEvents();
         } catch (...) {
             // This will ensure we don't do certain cleanups that are awaiting other tasks,
             // which could result in deadlocks on exit in case we encountered a serious problem.