From 993ffb56f44ac4adb7d24b5dc9e28e3e0d946167 Mon Sep 17 00:00:00 2001
From: Billy Laws <blaws05@gmail.com>
Date: Sun, 9 Oct 2022 12:55:56 +0100
Subject: [PATCH] Avoid waiting on texture/buffer fence with trapMutex locked

This could cause deadlocks under certain circumstances by preventing the GPFIFO from making any progress while also waiting on it at the same time.
---
 app/src/main/cpp/skyline/gpu/buffer.cpp       | 28 +++++++++++++------
 .../main/cpp/skyline/gpu/texture/texture.cpp  | 21 +++++++++++++-
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/app/src/main/cpp/skyline/gpu/buffer.cpp b/app/src/main/cpp/skyline/gpu/buffer.cpp
index 3558d400..f48e7a9c 100644
--- a/app/src/main/cpp/skyline/gpu/buffer.cpp
+++ b/app/src/main/cpp/skyline/gpu/buffer.cpp
@@ -23,11 +23,23 @@ namespace skyline::gpu {
                 return;
 
             std::unique_lock stateLock{buffer->stateMutex};
-            if (buffer->AllCpuBackingWritesBlocked()) {
+            if (buffer->AllCpuBackingWritesBlocked() || buffer->dirtyState == DirtyState::GpuDirty) {
                 stateLock.unlock(); // If the lock isn't unlocked, a deadlock from threads waiting on the other lock can occur
 
                 // If this mutex would cause other callbacks to be blocked then we should block on this mutex in advance
-                std::scoped_lock lock{*buffer};
+                std::shared_ptr<FenceCycle> waitCycle{};
+                do {
+                    if (waitCycle)
+                        waitCycle->Wait();
+
+                    std::scoped_lock lock{*buffer};
+                    if (waitCycle && buffer->cycle == waitCycle) {
+                        buffer->cycle = {};
+                        waitCycle = {};
+                    } else {
+                        waitCycle = buffer->cycle;
+                    }
+                } while (waitCycle);
             }
         }, [weakThis] {
             TRACE_EVENT("gpu", "Buffer::ReadTrap");
@@ -47,6 +59,9 @@ namespace skyline::gpu {
             if (!lock)
                 return false;
 
+            if (buffer->cycle)
+                return false;
+
             buffer->SynchronizeGuest(true); // We can skip trapping since the caller will do it
             return true;
         }, [weakThis] {
@@ -69,7 +84,9 @@ namespace skyline::gpu {
             if (!lock)
                 return false;
 
-            buffer->WaitOnFence();
+            if (buffer->cycle)
+                return false;
+
             buffer->SynchronizeGuest(true); // We need to assume the buffer is dirty since we don't know what the guest is writing
             buffer->dirtyState = DirtyState::CpuDirty;
 
@@ -128,8 +145,6 @@ namespace skyline::gpu {
     void Buffer::WaitOnFence() {
         TRACE_EVENT("gpu", "Buffer::WaitOnFence");
 
-        std::scoped_lock lock{stateMutex};
-
         if (cycle) {
             cycle->Wait();
             cycle = nullptr;
@@ -137,8 +152,6 @@ namespace skyline::gpu {
     }
 
     bool Buffer::PollFence() {
-        std::scoped_lock lock{stateMutex};
-
         if (!cycle)
             return true;
 
@@ -219,7 +232,6 @@ namespace skyline::gpu {
     }
 
     void Buffer::Read(bool isFirstUsage, const std::function<void()> &flushHostCallback, span<u8> data, vk::DeviceSize offset) {
-        std::scoped_lock lock{stateMutex};
         if (dirtyState == DirtyState::GpuDirty)
             SynchronizeGuestImmediate(isFirstUsage, flushHostCallback);
 
diff --git a/app/src/main/cpp/skyline/gpu/texture/texture.cpp b/app/src/main/cpp/skyline/gpu/texture/texture.cpp
index ec758b0f..c306132c 100644
--- a/app/src/main/cpp/skyline/gpu/texture/texture.cpp
+++ b/app/src/main/cpp/skyline/gpu/texture/texture.cpp
@@ -159,7 +159,19 @@ namespace skyline::gpu {
                 stateLock.unlock(); // If the lock isn't unlocked, a deadlock from threads waiting on the other lock can occur
 
                 // If this mutex would cause other callbacks to be blocked then we should block on this mutex in advance
-                std::scoped_lock lock{*texture};
+                std::shared_ptr<FenceCycle> waitCycle{};
+                do {
+                    if (waitCycle)
+                        waitCycle->Wait();
+
+                    std::scoped_lock lock{*texture};
+                    if (waitCycle && texture->cycle == waitCycle) {
+                        texture->cycle = {};
+                        waitCycle = {};
+                    } else {
+                        waitCycle = texture->cycle;
+                    }
+                } while (waitCycle);
             }
         }, [weakThis] {
             TRACE_EVENT("gpu", "Texture::ReadTrap");
@@ -179,6 +191,9 @@ namespace skyline::gpu {
             if (!lock)
                 return false;
 
+            if (texture->cycle)
+                return false;
+
             texture->SynchronizeGuest(false, true); // We can skip trapping since the caller will do it
             return true;
         }, [weakThis] {
@@ -200,6 +215,10 @@ namespace skyline::gpu {
             std::unique_lock lock{*texture, std::try_to_lock};
             if (!lock)
                 return false;
+
+            if (texture->cycle)
+                return false;
+
             texture->SynchronizeGuest(true, true); // We need to assume the texture is dirty since we don't know what the guest is writing
             return true;
         });