From 745d809e0701b5a2bb99d48ac36a5e2087a6dee6 Mon Sep 17 00:00:00 2001
From: PixelyIon <pixelyion@protonmail.com>
Date: Wed, 13 Jul 2022 22:19:37 +0530
Subject: [PATCH] Fix `Buffer::SynchronizeGuest` Non-Blocking Behavior

The buffer's non-blocking behavior could lead to an invalid state where the dirty state doesn't adequately represent the buffer's true state, the check has now been moved inside the CAS loop as its behavior changes depending on the dirty state. In addition, `SynchronizeGuest` returns a boolean denoting if the synchronization was successful now to make code flows depending on non-blocking synchronization cleaner.
---
 app/src/main/cpp/skyline/gpu/buffer.cpp | 21 ++++++++++-----------
 app/src/main/cpp/skyline/gpu/buffer.h   |  3 ++-
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/app/src/main/cpp/skyline/gpu/buffer.cpp b/app/src/main/cpp/skyline/gpu/buffer.cpp
index 45e5af1d..3a1b0d5c 100644
--- a/app/src/main/cpp/skyline/gpu/buffer.cpp
+++ b/app/src/main/cpp/skyline/gpu/buffer.cpp
@@ -160,31 +160,32 @@ namespace skyline::gpu {
         gpu.state.nce->RetrapRegions(*trapHandle, !rwTrap); // Trap any future CPU reads (optionally) + writes to this buffer
     }
 
-    void Buffer::SynchronizeGuest(bool skipTrap, bool nonBlocking, bool setDirty) {
+    bool Buffer::SynchronizeGuest(bool skipTrap, bool nonBlocking, bool setDirty) {
         if (!guest)
-            return;
+            return false;
 
         auto currentState{dirtyState.load(std::memory_order_relaxed)};
         do {
             if (currentState == DirtyState::CpuDirty || (currentState == DirtyState::Clean && setDirty))
-                return; // If the buffer is synchronized (Clean/CpuDirty), there is no need to synchronize it
+                return true; // If the buffer is synchronized (Clean/CpuDirty), there is no need to synchronize it
+            else if (currentState == DirtyState::GpuDirty && nonBlocking && !PollFence())
+                return false; // If the buffer is GPU dirty and the fence is not signalled then we can't block
         } while (!dirtyState.compare_exchange_strong(currentState, setDirty ? DirtyState::CpuDirty : DirtyState::Clean, std::memory_order_relaxed));
 
-        if (nonBlocking && !PollFence())
-            return;
-
         TRACE_EVENT("gpu", "Buffer::SynchronizeGuest");
 
         if (!skipTrap)
             gpu.state.nce->RetrapRegions(*trapHandle, true);
 
         if (setDirty && currentState == DirtyState::Clean)
-            return; // If the texture was simply transitioned from Clean to CpuDirty, there is no need to synchronize it
+            return true; // If the texture was simply transitioned from Clean to CpuDirty, there is no need to synchronize it
 
         if (!nonBlocking)
             WaitOnFence();
 
         std::memcpy(mirror.data(), backing.data(), mirror.size());
+
+        return true;
     }
 
     void Buffer::SynchronizeGuestImmediate(bool isFirstUsage, const std::function<void()> &flushHostCallback) {
@@ -231,10 +232,8 @@ namespace skyline::gpu {
     }
 
     std::pair<u64, span<u8>> Buffer::AcquireCurrentSequence() {
-        SynchronizeGuest(false, true); // First try to remove GPU dirtiness by doing an immediate sync and taking a quick shower
-
-        if (dirtyState == DirtyState::GpuDirty)
-            // Bail out if buffer is GPU dirty - since we don't know the contents ahead of time the sequence is indeterminate
+        if (!SynchronizeGuest(false, true))
+            // Bail out if buffer cannot be synced, we don't know the contents ahead of time so the sequence is indeterminate
             return {};
 
         SynchronizeHost(); // Ensure that the returned mirror is fully up-to-date by performing a CPU -> GPU sync
diff --git a/app/src/main/cpp/skyline/gpu/buffer.h b/app/src/main/cpp/skyline/gpu/buffer.h
index f7333833..7c533ed3 100644
--- a/app/src/main/cpp/skyline/gpu/buffer.h
+++ b/app/src/main/cpp/skyline/gpu/buffer.h
@@ -234,9 +234,10 @@ namespace skyline::gpu {
          * @param skipTrap If true, setting up a CPU trap will be skipped and the dirty state will be Clean/CpuDirty
          * @param nonBlocking If true, the call will return immediately if the fence is not signalled, skipping the sync
          * @param setDirty If true, the buffer will be marked as CpuDirty rather than Clean
+         * @return If the buffer's contents were successfully synchronized, this'll only be false on non-blocking operations or lack of a guest buffer
          * @note The buffer **must** be locked prior to calling this
          */
-        void SynchronizeGuest(bool skipTrap = false, bool nonBlocking = false, bool setDirty = false);
+        bool SynchronizeGuest(bool skipTrap = false, bool nonBlocking = false, bool setDirty = false);
 
         /**
          * @brief Synchronizes the guest buffer with the host buffer immediately, flushing GPU work if necessary