From 1feb56e4783b0b81f8c2384324f643e3555cb373 Mon Sep 17 00:00:00 2001
From: Leonard Hecker <lhecker@microsoft.com>
Date: Wed, 17 Jul 2024 17:37:26 +0200
Subject: [PATCH] Fix cursor inheritance (#17576)

---
 src/host/VtIo.cpp                               | 13 ++++++++-----
 src/inc/til/atomic.h                            | 14 ++++++++++----
 src/terminal/parser/InputStateMachineEngine.cpp |  4 ++--
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/src/host/VtIo.cpp b/src/host/VtIo.cpp
index 1511b7e34b4..71f170aed3a 100644
--- a/src/host/VtIo.cpp
+++ b/src/host/VtIo.cpp
@@ -183,6 +183,11 @@ bool VtIo::IsUsingVt() const
         CATCH_RETURN();
     }
 
+    if (_pVtInputThread)
+    {
+        LOG_IF_FAILED(_pVtInputThread->Start());
+    }
+
     // MSFT: 15813316
     // If the terminal application wants us to inherit the cursor position,
     //  we're going to emit a VT sequence to ask for the cursor position, then
@@ -193,6 +198,9 @@ bool VtIo::IsUsingVt() const
     {
         _lookingForCursorPosition = false;
         LOG_IF_FAILED(_pVtRenderEngine->RequestCursor());
+
+        // Allow the input thread to momentarily gain the console lock.
+        const auto suspension = g.getConsoleInformation().SuspendLock();
         _pVtInputThread->WaitUntilDSR(3000);
     }
 
@@ -202,11 +210,6 @@ bool VtIo::IsUsingVt() const
     // this sequence, it'll just ignore it.
     LOG_IF_FAILED(_pVtRenderEngine->RequestWin32Input());
 
-    if (_pVtInputThread)
-    {
-        LOG_IF_FAILED(_pVtInputThread->Start());
-    }
-
     if (_pPtySignalInputThread)
     {
         // Let the signal thread know that the console is connected.
diff --git a/src/inc/til/atomic.h b/src/inc/til/atomic.h
index 26fdd555f23..ca9ca9a25e2 100644
--- a/src/inc/til/atomic.h
+++ b/src/inc/til/atomic.h
@@ -5,23 +5,29 @@
 
 namespace til
 {
+    // Similar to std::atomic<T>::wait, but slightly faster and with the ability to specify a timeout.
+    // Returns false on failure, which is pretty much always a timeout. (We prevent invalid arguments by taking references.)
     template<typename T>
-    inline void atomic_wait(const std::atomic<T>& atomic, const T& current, DWORD waitMilliseconds = INFINITE) noexcept
+    bool atomic_wait(const std::atomic<T>& atomic, const T& current, DWORD waitMilliseconds = INFINITE) noexcept
     {
         static_assert(sizeof(atomic) == sizeof(current));
 #pragma warning(suppress : 26492) // Don't use const_cast to cast away const or volatile
-        WaitOnAddress(const_cast<std::atomic<T>*>(&atomic), const_cast<T*>(&current), sizeof(current), waitMilliseconds);
+        return WaitOnAddress(const_cast<std::atomic<T>*>(&atomic), const_cast<T*>(&current), sizeof(current), waitMilliseconds);
     }
 
+    // Wakes at most one of the threads waiting on the atomic via atomic_wait().
+    // Don't use this with std::atomic<T>::wait, because it's not guaranteed to work in the future.
     template<typename T>
-    inline void atomic_notify_one(const std::atomic<T>& atomic) noexcept
+    void atomic_notify_one(const std::atomic<T>& atomic) noexcept
     {
 #pragma warning(suppress : 26492) // Don't use const_cast to cast away const or volatile
         WakeByAddressSingle(const_cast<std::atomic<T>*>(&atomic));
     }
 
+    // Wakes all threads waiting on the atomic via atomic_wait().
+    // Don't use this with std::atomic<T>::wait, because it's not guaranteed to work in the future.
     template<typename T>
-    inline void atomic_notify_all(const std::atomic<T>& atomic) noexcept
+    void atomic_notify_all(const std::atomic<T>& atomic) noexcept
     {
 #pragma warning(suppress : 26492) // Don't use const_cast to cast away const or volatile
         WakeByAddressAll(const_cast<std::atomic<T>*>(&atomic));
diff --git a/src/terminal/parser/InputStateMachineEngine.cpp b/src/terminal/parser/InputStateMachineEngine.cpp
index debcbe7fe74..645ff33a1dc 100644
--- a/src/terminal/parser/InputStateMachineEngine.cpp
+++ b/src/terminal/parser/InputStateMachineEngine.cpp
@@ -105,11 +105,11 @@ InputStateMachineEngine::InputStateMachineEngine(std::unique_ptr<IInteractDispat
 
 void InputStateMachineEngine::WaitUntilDSR(DWORD timeout) const noexcept
 {
+    // atomic_wait() returns false when the timeout expires.
     // Technically we should decrement the timeout with each iteration,
     // but I suspect infinite spurious wake-ups are a theoretical problem.
-    while (_lookingForDSR.load(std::memory_order::relaxed))
+    while (_lookingForDSR.load(std::memory_order::relaxed) && til::atomic_wait(_lookingForDSR, true, timeout))
     {
-        til::atomic_wait(_lookingForDSR, true, timeout);
     }
 }