Fix startup race of resizing ConPTY (#10449)

Fix startup race of resizing ConPTY

- Depending on what the timing and ordering is of the message coming in
  from the signal thread, it may be applied to the startup structure
  after the I/O thread has begun initializing the console buffer
  structures but before it has signaled that it is done and the signal
  thread is ready to make changes directly. This likely happens because
  the end of the I/O thread setup has a weird unlock/lock jog for the
  input thread and the signal thread might have been scheduled in the
  middle of it.
- My resolution here is to ensure that the signal thread just keeps
  storing the latest resize message until it is told that everything is
  initialized. Whomever comes in to tell the signal thread this
  information (under lock) will pickup and run the resize if one came in
  before everything was ready. This should resolve the race.

## Validation Steps Performed
- o-sdn-o confirms this resolves their issue

Closes #10400

src/host/ConsoleArguments.cpp

@@ -103,10 +103,7 @@ ConsoleArguments::ConsoleArguments(const std::wstring& commandline,
                                    const HANDLE hStdOut) :
     _commandline(commandline),
     _vtInHandle(hStdIn),
-    _vtOutHandle(hStdOut),
-    _receivedEarlySizeChange{ false },
-    _originalWidth{ -1 },
-    _originalHeight{ -1 }
+    _vtOutHandle(hStdOut)
 {
     _clientCommandline = L"";
     _vtMode = L"";
@@ -144,7 +141,6 @@ ConsoleArguments& ConsoleArguments::operator=(const ConsoleArguments& other)
         _width = other._width;
         _height = other._height;
         _inheritCursor = other._inheritCursor;
-        _receivedEarlySizeChange = other._receivedEarlySizeChange;
         _runAsComServer = other._runAsComServer;
         _forceNoHandoff = other._forceNoHandoff;
     }
@@ -668,33 +664,6 @@ bool ConsoleArguments::IsWin32InputModeEnabled() const
     return _win32InputMode;
 }
 
-// Method Description:
-// - Tell us to use a different size than the one parsed as the size of the
-//      console. This is called by the PtySignalInputThread when it receives a
-//      resize before the first client has connected. Because there's no client,
-//      there's also no buffer yet, so it has nothing to resize.
-//      However, we shouldn't just discard that first resize message. Instead,
-//      store it in here, so we can use the value when the first client does connect.
-// Arguments:
-// - dimensions: the new size in characters of the conpty buffer & viewport.
-// Return Value:
-// - <none>
-void ConsoleArguments::SetExpectedSize(COORD dimensions) noexcept
-{
-    _width = dimensions.X;
-    _height = dimensions.Y;
-    // Stash away the original values we parsed when this is called.
-    // This is to help debugging - if the signal thread DOES change these values,
-    //      we can still recover what was given to us on the commandline.
-    if (!_receivedEarlySizeChange)
-    {
-        _originalWidth = _width;
-        _originalHeight = _height;
-        // Mark that we've changed size from what our commandline values were
-        _receivedEarlySizeChange = true;
-    }
-}
-
 #ifdef UNIT_TESTING
 // Method Description:
 // - This is a test helper method. It can be used to trick us into thinking

src/host/ConsoleArguments.hpp

@@ -56,8 +56,6 @@ class ConsoleArguments
     bool IsResizeQuirkEnabled() const;
     bool IsWin32InputModeEnabled() const;
 
-    void SetExpectedSize(COORD dimensions) noexcept;
-
 #ifdef UNIT_TESTING
     void EnableConptyModeForTests();
 #endif
@@ -113,9 +111,6 @@ class ConsoleArguments
         _signalHandle(signalHandle),
         _inheritCursor(inheritCursor),
         _resizeQuirk(false),
-        _receivedEarlySizeChange{ false },
-        _originalWidth{ -1 },
-        _originalHeight{ -1 },
         _runAsComServer{ runAsComServer }
     {
     }
@@ -146,10 +141,6 @@ class ConsoleArguments
     bool _resizeQuirk{ false };
     bool _win32InputMode{ false };
 
-    bool _receivedEarlySizeChange;
-    short _originalWidth;
-    short _originalHeight;
-
     [[nodiscard]] HRESULT _GetClientCommandline(_Inout_ std::vector<std::wstring>& args,
                                                 const size_t index,
                                                 const bool skipFirst);

src/host/PtySignalInputThread.cpp

@@ -10,14 +10,6 @@
 #include "../interactivity/inc/ServiceLocator.hpp"
 #include "../terminal/adapter/DispatchCommon.hpp"
 
-#define PTY_SIGNAL_RESIZE_WINDOW 8u
-
-struct PTY_SIGNAL_RESIZE
-{
-    unsigned short sx;
-    unsigned short sy;
-};
-
 using namespace Microsoft::Console;
 using namespace Microsoft::Console::Interactivity;
 using namespace Microsoft::Console::VirtualTerminal;
@@ -63,13 +55,19 @@ DWORD WINAPI PtySignalInputThread::StaticThreadProc(_In_ LPVOID lpParameter)
 //      do something with the messages we receive now. Before this is set, there
 //      is no guarantee that a client has attached, so most parts of the console
 //      (in and screen buffers) haven't yet been initialized.
+// - NOTE: Call under LockConsole() to ensure other threads have an opportunity
+//         to set early-work state.
 // Arguments:
 // - <none>
 // Return Value:
 // - <none>
 void PtySignalInputThread::ConnectConsole() noexcept
 {
     _consoleConnected = true;
+    if (_earlyResize)
+    {
+        _DoResizeWindow(*_earlyResize);
+    }
 }
 
 // Method Description:
@@ -79,38 +77,30 @@ void PtySignalInputThread::ConnectConsole() noexcept
 // - Otherwise it may cause an application termination and never return.
 [[nodiscard]] HRESULT PtySignalInputThread::_InputThread()
 {
-    unsigned short signalId;
+    PtySignal signalId;
     while (_GetData(&signalId, sizeof(signalId)))
     {
         switch (signalId)
         {
-        case PTY_SIGNAL_RESIZE_WINDOW:
+        case PtySignal::ResizeWindow:
         {
-            PTY_SIGNAL_RESIZE resizeMsg = { 0 };
+            ResizeWindowData resizeMsg = { 0 };
             _GetData(&resizeMsg, sizeof(resizeMsg));
 
             LockConsole();
             auto Unlock = wil::scope_exit([&] { UnlockConsole(); });
+
             // If the client app hasn't yet connected, stash the new size in the launchArgs.
             // We'll later use the value in launchArgs to set up the console buffer
+            // We must be under lock here to ensure that someone else doesn't come in
+            // and set with `ConnectConsole` while we're looking and modifying this.
             if (!_consoleConnected)
             {
-                short sColumns = 0;
-                short sRows = 0;
-                if (SUCCEEDED(UShortToShort(resizeMsg.sx, &sColumns)) &&
-                    SUCCEEDED(UShortToShort(resizeMsg.sy, &sRows)) &&
-                    (sColumns > 0 && sRows > 0))
-                {
-                    ServiceLocator::LocateGlobals().launchArgs.SetExpectedSize({ sColumns, sRows });
-                }
-                break;
+                _earlyResize = resizeMsg;
             }
             else
             {
-                if (DispatchCommon::s_ResizeWindow(*_pConApi, resizeMsg.sx, resizeMsg.sy))
-                {
-                    DispatchCommon::s_SuppressResizeRepaint(*_pConApi);
-                }
+                _DoResizeWindow(resizeMsg);
             }
 
             break;
@@ -124,6 +114,20 @@ void PtySignalInputThread::ConnectConsole() noexcept
     return S_OK;
 }
 
+// Method Description:
+// - Dispatches a resize window message to the rest of the console code
+// Arguments:
+// - data - Packet information containing width/height (size) information
+// Return Value:
+// - <none>
+void PtySignalInputThread::_DoResizeWindow(const ResizeWindowData& data)
+{
+    if (DispatchCommon::s_ResizeWindow(*_pConApi, data.sx, data.sy))
+    {
+        DispatchCommon::s_SuppressResizeRepaint(*_pConApi);
+    }
+}
+
 // Method Description:
 // - Retrieves bytes from the file stream and exits or throws errors should the pipe state
 //   be compromised.

src/host/PtySignalInputThread.hpp

@@ -33,14 +33,27 @@ namespace Microsoft::Console
         void ConnectConsole() noexcept;
 
     private:
+        enum class PtySignal : unsigned short
+        {
+            ResizeWindow = 8
+        };
+
+        struct ResizeWindowData
+        {
+            unsigned short sx;
+            unsigned short sy;
+        };
+
         [[nodiscard]] HRESULT _InputThread();
         bool _GetData(_Out_writes_bytes_(cbBuffer) void* const pBuffer, const DWORD cbBuffer);
+        void _DoResizeWindow(const ResizeWindowData& data);
         void _Shutdown();
 
         wil::unique_hfile _hFile;
         wil::unique_handle _hThread;
         DWORD _dwThreadId;
         bool _consoleConnected;
+        std::optional<ResizeWindowData> _earlyResize;
         std::unique_ptr<Microsoft::Console::VirtualTerminal::ConGetSet> _pConApi;
     };
 }