From d52ba8c3fe93f90e7754e641494c1d3bd81bec16 Mon Sep 17 00:00:00 2001
From: Vlad Zahorodnii <vlad.zahorodnii@kde.org>
Date: Tue, 7 Nov 2023 17:07:36 +0200
Subject: [PATCH] wayland: Fix transaction cleanup with destroyed surfaces

When sorting surfaces in the ancestor order we need to ignore null
surfaces.

In addition to that, we also need to properly handle the case where
a transaction with dependencies is unlocked and it contains null
surfaces.

For example, if there are three transactions A -> B -> C, and the B
transaction is unlocked, we cannot apply it until transaction A is
applied. The readiness check is based on checking the first pending
transaction of the surface. But if the surface is null, the check
will be skipped, which is not ideal as transaction B can be applied
before transaction A now. To address that, this change makes every
transaction entry remember the previous transaction. With that, the
readiness check can be performed even if the surface has been destroyed.

BUG: 475648
---
 src/wayland/transaction.cpp | 27 +++++++++++++++++----------
 src/wayland/transaction.h   |  5 +++++
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/src/wayland/transaction.cpp b/src/wayland/transaction.cpp
index b6364220c3..93004ba863 100644
--- a/src/wayland/transaction.cpp
+++ b/src/wayland/transaction.cpp
@@ -100,16 +100,9 @@ bool Transaction::isReady() const
         return false;
     }
 
-    for (const TransactionEntry &entry : m_entries) {
-        if (!entry.surface) {
-            continue;
-        }
-        if (entry.surface->firstTransaction() != this) {
-            return false;
-        }
-    }
-
-    return true;
+    return std::none_of(m_entries.cbegin(), m_entries.cend(), [](const TransactionEntry &entry) {
+        return entry.previousTransaction;
+    });
 }
 
 Transaction *Transaction::next(SurfaceInterface *surface) const
@@ -195,6 +188,13 @@ void Transaction::apply()
 {
     // Sort surfaces so descendants come first, then their ancestors.
     std::sort(m_entries.begin(), m_entries.end(), [](const TransactionEntry &a, const TransactionEntry &b) {
+        if (!a.surface) {
+            return false;
+        }
+        if (!b.surface) {
+            return true;
+        }
+
         if (isAncestor(a.surface, b.surface)) {
             return true;
         }
@@ -221,6 +221,12 @@ void Transaction::apply()
         }
 
         if (entry.nextTransaction) {
+            for (TransactionEntry &otherEntry : entry.nextTransaction->m_entries) {
+                if (otherEntry.previousTransaction == this) {
+                    otherEntry.previousTransaction = nullptr;
+                    break;
+                }
+            }
             entry.nextTransaction->tryApply();
         }
     }
@@ -258,6 +264,7 @@ void Transaction::commit()
             entry.surface->setFirstTransaction(this);
         }
 
+        entry.previousTransaction = entry.surface->lastTransaction();
         entry.surface->setLastTransaction(this);
     }
 
diff --git a/src/wayland/transaction.h b/src/wayland/transaction.h
index c03bb91986..d138dc8422 100644
--- a/src/wayland/transaction.h
+++ b/src/wayland/transaction.h
@@ -33,6 +33,11 @@ struct TransactionEntry
      */
     QPointer<SurfaceInterface> surface;
 
+    /**
+     * The previous transaction that this transaction depends on.
+     */
+    Transaction *previousTransaction = nullptr;
+
     /**
      * Next transaction that is going to affect the surface.
      */