Improve reliability of VT responses (#17786)

* Repurposes `_sendInputToConnection` to send output to the connection
  no matter whether the terminal is read-only or not.
  Now `SendInput` is the function responsible for the UI handling.
* Buffers responses in a VT string into a single string
  before sending it as a response all at once.

This reduces the chances for the UI thread to insert cursor positions
and similar into the input pipe, because we're not constantly unlocking
the terminal lock anymore for every response. The only way now that
unrelated inputs are inserted into the input pipe is because the VT
requests (e.g. DA1, DSR, etc.) are broken up across >1 reads.

This also fixes VT responses in read-only panes.

Closes #17775

## Validation Steps Performed
* Repeatedly run `echo ^[[c` in cmd.
  DA1 responses don't stack & always stay the same ✅
* Run nvim in WSL. Doesn't deadlock when pasting 1MB. ✅
* Run the repro from #17775, which requests a ton of OSC 4
  (color palette) responses. Jiggle the cursor on top of the window.
  Responses never get split up. ✅

Author: lhecker

src/cascadia/TerminalControl/ControlCore.cpp

@@ -98,7 +98,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         Connection(connection);
 
         _terminal->SetWriteInputCallback([this](std::wstring_view wstr) {
-            _sendInputToConnection(wstr);
+            _pendingResponses.append(wstr);
         });
 
         // GH#8969: pre-seed working directory to prevent potential races
@@ -419,6 +419,17 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     // Return Value:
     // - <none>
     void ControlCore::_sendInputToConnection(std::wstring_view wstr)
+    {
+        _connection.WriteInput(winrt_wstring_to_array_view(wstr));
+    }
+
+    // Method Description:
+    // - Writes the given sequence as input to the active terminal connection,
+    // Arguments:
+    // - wstr: the string of characters to write to the terminal connection.
+    // Return Value:
+    // - <none>
+    void ControlCore::SendInput(const std::wstring_view wstr)
     {
         if (wstr.empty())
         {
@@ -435,21 +446,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         }
         else
         {
-            _connection.WriteInput(winrt_wstring_to_array_view(wstr));
+            _sendInputToConnection(wstr);
         }
     }
 
-    // Method Description:
-    // - Writes the given sequence as input to the active terminal connection,
-    // Arguments:
-    // - wstr: the string of characters to write to the terminal connection.
-    // Return Value:
-    // - <none>
-    void ControlCore::SendInput(const std::wstring_view wstr)
-    {
-        _sendInputToConnection(wstr);
-    }
-
     bool ControlCore::SendCharEvent(const wchar_t ch,
                                     const WORD scanCode,
                                     const ::Microsoft::Terminal::Core::ControlKeyStates modifiers)
@@ -485,7 +485,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         }
         if (out)
         {
-            _sendInputToConnection(*out);
+            SendInput(*out);
             return true;
         }
         return false;
@@ -643,7 +643,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         }
         if (out)
         {
-            _sendInputToConnection(*out);
+            SendInput(*out);
             return true;
         }
         return false;
@@ -662,7 +662,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         }
         if (out)
         {
-            _sendInputToConnection(*out);
+            SendInput(*out);
             return true;
         }
         return false;
@@ -1412,7 +1412,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         }
 
         // It's important to not hold the terminal lock while calling this function as sending the data may take a long time.
-        _sendInputToConnection(filtered);
+        SendInput(filtered);
 
         const auto lock = _terminal->LockForWriting();
         _terminal->ClearSelection();
@@ -2107,7 +2107,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                     // Sending input requires that we're unlocked, because
                     // writing the input pipe may block indefinitely.
                     const auto suspension = _terminal->SuspendLock();
-                    _sendInputToConnection(buffer);
+                    SendInput(buffer);
                 }
             }
         }
@@ -2164,6 +2164,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                 _terminal->Write(hstr);
             }
 
+            if (!_pendingResponses.empty())
+            {
+                _sendInputToConnection(_pendingResponses);
+                _pendingResponses.clear();
+            }
+
             // Start the throttled update of where our hyperlinks are.
             const auto shared = _shared.lock_shared();
             if (shared->outputIdle)
@@ -2480,9 +2486,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         }
         if (out && !out->empty())
         {
-            // _sendInputToConnection() asserts that we aren't in focus mode,
-            // but window focus events are always fine to send.
-            _connection.WriteInput(winrt_wstring_to_array_view(*out));
+            _sendInputToConnection(*out);
         }
     }
 

src/cascadia/TerminalControl/ControlCore.h

@@ -319,6 +319,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         winrt::com_ptr<ControlSettings> _settings{ nullptr };
 
         std::shared_ptr<::Microsoft::Terminal::Core::Terminal> _terminal{ nullptr };
+        std::wstring _pendingResponses;
 
         // NOTE: _renderEngine must be ordered before _renderer.
         //

src/cascadia/TerminalCore/TerminalApi.cpp

@@ -24,7 +24,6 @@ void Terminal::ReturnResponse(const std::wstring_view response)
 {
     if (_pfnWriteInput && !response.empty())
     {
-        const auto suspension = _readWriteLock.suspend();
         _pfnWriteInput(response);
     }
 }