Add support for start /B and FreeConsole (#14544)

2 new ConPTY APIs were added as part of this commit:
* `ClosePseudoConsoleTimeout`
  Complements `ClosePseudoConsole`, allowing users to override the `INFINITE`
  wait for OpenConsole to exit with any arbitrary timeout, including 0.
* `ConptyReleasePseudoConsole`
  This releases the `\Reference` handle held by the `HPCON`. While it makes
  launching further PTY clients via `PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE`
  impossible, it does allow conhost/OpenConsole to exit naturally once all
  console clients have disconnected. This makes it unnecessary to having to
  monitor the exit of the spawned shell/application, just to then call
  `ClosePseudoConsole`, while carefully continuing to read from the output
  pipe. Instead users can now just read from the output pipe until it is
  closed, knowing for sure that no more data will come or clients connect.
  This is especially useful in combination with `ClosePseudoConsoleTimeout`
  and a timeout of 0, to allow conhost/OpenConsole to exit asynchronously.

These new APIs are used to fix an array of bugs around Windows Terminal exiting
either too early or too late. They also make usage of the ConPTY API simpler in
most situations (when spawning a single application and waiting for it to exit).

Depends on #13882, #14041, #14160, #14282

Closes #4564
Closes #14416
Closes MSFT-42244182

## Validation Steps Performed
* Calling `FreeConsole` in a handoff'd application closes the tab ✅
* Create a .bat file containing only `start /B cmd.exe`.
  If WT stable is the default terminal the tab closes instantly ✅
  With these changes included the tab stays open with a cmd.exe prompt ✅
* New ConPTY tests ✅

.github/actions/spelling/expect/expect.txt

@@ -817,6 +817,7 @@ HORZ
 hostable
 hostlib
 HPA
+hpcon
 HPCON
 hpj
 HPR

src/cascadia/TerminalConnection/ConptyConnection.cpp

@@ -2,17 +2,17 @@
 // Licensed under the MIT license.
 
 #include "pch.h"
-
 #include "ConptyConnection.h"
 
+#include <conpty-static.h>
 #include <winternl.h>
 
-#include "ConptyConnection.g.cpp"
 #include "CTerminalHandoff.h"
-
-#include "../../types/inc/utils.hpp"
-#include "../../types/inc/Environment.hpp"
 #include "LibraryResources.h"
+#include "../../types/inc/Environment.hpp"
+#include "../../types/inc/utils.hpp"
+
+#include "ConptyConnection.g.cpp"
 
 using namespace ::Microsoft::Console;
 using namespace std::string_view_literals;
@@ -24,13 +24,9 @@ static constexpr auto _errorFormat = L"{0} ({0:#010x})"sv;
 // There is a number of ways that the Conpty connection can be terminated (voluntarily or not):
 // 1. The connection is Close()d
 // 2. The pseudoconsole or process cannot be spawned during Start()
-// 3. The client process exits with a code.
-//    (Successful (0) or any other code)
-// 4. The read handle is terminated.
-//    (This usually happens when the pseudoconsole host crashes.)
+// 3. The read handle is terminated (when OpenConsole exits)
 // In each of these termination scenarios, we need to be mindful of tripping the others.
-// Closing the pseudoconsole in response to the client exiting (3) can trigger (4).
-// Close() (1) will cause the automatic triggering of (3) and (4).
+// Close() (1) will cause the automatic triggering of (3).
 // In a lot of cases, we use the connection state to stop "flapping."
 //
 // To figure out where we handle these, search for comments containing "EXIT POINT"
@@ -380,6 +376,8 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
             }
         }
 
+        THROW_IF_FAILED(ConptyReleasePseudoConsole(_hPC.get()));
+
         _startTime = std::chrono::high_resolution_clock::now();
 
         // Create our own output handling thread
@@ -404,19 +402,6 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
 
         LOG_IF_FAILED(SetThreadDescription(_hOutputThread.get(), L"ConptyConnection Output Thread"));
 
-        _clientExitWait.reset(CreateThreadpoolWait(
-            [](PTP_CALLBACK_INSTANCE /*callbackInstance*/, PVOID context, PTP_WAIT /*wait*/, TP_WAIT_RESULT /*waitResult*/) noexcept {
-                const auto pInstance = static_cast<ConptyConnection*>(context);
-                if (pInstance)
-                {
-                    pInstance->_ClientTerminated();
-                }
-            },
-            this,
-            nullptr));
-
-        SetThreadpoolWait(_clientExitWait.get(), _piClient.hProcess, nullptr);
-
         _transitionToState(ConnectionState::Connected);
     }
     catch (...)
@@ -466,34 +451,17 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
 
     // Method Description:
     // - called when the client application (not necessarily its pty) exits for any reason
-    void ConptyConnection::_ClientTerminated() noexcept
+    void ConptyConnection::_LastConPtyClientDisconnected() noexcept
     try
     {
-        if (_isStateAtOrBeyond(ConnectionState::Closing))
-        {
-            // This termination was expected.
-            return;
-        }
-
-        // EXIT POINT
         DWORD exitCode{ 0 };
         GetExitCodeProcess(_piClient.hProcess, &exitCode);
 
         // Signal the closing or failure of the process.
-        // Load bearing. Terminating the pseudoconsole will make the output thread exit unexpectedly,
-        // so we need to signal entry into the correct closing state before we do that.
-        _transitionToState(exitCode == 0 ? ConnectionState::Closed : ConnectionState::Failed);
-
-        // Close the pseudoconsole and wait for all output to drain.
-        _hPC.reset();
-        if (auto localOutputThreadHandle = std::move(_hOutputThread))
-        {
-            LOG_LAST_ERROR_IF(WAIT_FAILED == WaitForSingleObject(localOutputThreadHandle.get(), INFINITE));
-        }
-
+        // exitCode might be STILL_ACTIVE if a client has called FreeConsole() and
+        // thus caused the tab to close, even though the CLI app is still running.
+        _transitionToState(exitCode == 0 || exitCode == STILL_ACTIVE ? ConnectionState::Closed : ConnectionState::Failed);
         _indicateExitWithStatus(exitCode);
-
-        _piClient.reset();
     }
     CATCH_LOG()
 
@@ -564,43 +532,46 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
     void ConptyConnection::Close() noexcept
     try
     {
-        const bool isClosing = _transitionToState(ConnectionState::Closing);
-        if (isClosing || _isStateAtOrBeyond(ConnectionState::Closed))
-        {
-            // EXIT POINT
-
-            // _clientExitWait holds a CreateThreadpoolWait() which holds a weak reference to "this".
-            // This manual reset() ensures we wait for it to be teared down via WaitForThreadpoolWaitCallbacks().
-            _clientExitWait.reset();
+        _transitionToState(ConnectionState::Closing);
 
-            _hPC.reset(); // tear down the pseudoconsole (this is like clicking X on a console window)
+        // .reset()ing either of these two will signal ConPTY to send out a CTRL_CLOSE_EVENT to all attached clients.
+        // FYI: The other members of this class are concurrently read by the _hOutputThread
+        // thread running in the background and so they're not safe to be .reset().
+        _hPC.reset();
+        _inPipe.reset();
 
-            // CloseHandle() on pipes blocks until any current WriteFile()/ReadFile() has returned.
-            // CancelSynchronousIo prevents us from deadlocking ourselves.
-            // At this point in Close(), _inPipe won't be used anymore since the UI parts are torn down.
-            // _outPipe is probably still stuck in ReadFile() and might currently be written to.
-            if (_hOutputThread)
+        if (_hOutputThread)
+        {
+            // Loop around `CancelSynchronousIo()` just in case the signal to shut down was missed.
+            // This may happen if we called `CancelSynchronousIo()` while not being stuck
+            // in `ReadFile()` and if OpenConsole refuses to exit in a timely manner.
+            for (;;)
             {
+                // ConptyConnection::Close() blocks the UI thread, because `_TerminalOutputHandlers` might indirectly
+                // reference UI objects like `ControlCore`. CancelSynchronousIo() allows us to have the background
+                // thread exit as fast as possible by aborting any ongoing writes coming from OpenConsole.
                 CancelSynchronousIo(_hOutputThread.get());
-            }
 
-            _inPipe.reset(); // break the pipes
-            _outPipe.reset();
-
-            if (_hOutputThread)
-            {
                 // Waiting for the output thread to exit ensures that all pending _TerminalOutputHandlers()
                 // calls have returned and won't notify our caller (ControlCore) anymore. This ensures that
                 // we don't call a destroyed event handler asynchronously from a background thread (GH#13880).
-                LOG_LAST_ERROR_IF(WAIT_FAILED == WaitForSingleObject(_hOutputThread.get(), INFINITE));
-                _hOutputThread.reset();
-            }
+                const auto result = WaitForSingleObject(_hOutputThread.get(), 1000);
+                if (result == WAIT_OBJECT_0)
+                {
+                    break;
+                }
 
-            if (isClosing)
-            {
-                _transitionToState(ConnectionState::Closed);
+                LOG_LAST_ERROR();
             }
         }
+
+        // Now that the background thread is done, we can safely clean up the other system objects, without
+        // race conditions, or fear of deadlocking ourselves (e.g. by calling CloseHandle() on _outPipe).
+        _outPipe.reset();
+        _hOutputThread.reset();
+        _piClient.reset();
+
+        _transitionToState(ConnectionState::Closed);
     }
     CATCH_LOG()
 
@@ -657,15 +628,19 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
 
             if (readFail) // reading failed (we must check this first, because read will also be 0.)
             {
+                // EXIT POINT
                 const auto lastError = GetLastError();
-                if (lastError != ERROR_BROKEN_PIPE)
+                if (lastError == ERROR_BROKEN_PIPE)
+                {
+                    _LastConPtyClientDisconnected();
+                    return S_OK;
+                }
+                else
                 {
-                    // EXIT POINT
                     _indicateExitWithStatus(HRESULT_FROM_WIN32(lastError)); // print a message
                     _transitionToState(ConnectionState::Failed);
                     return gsl::narrow_cast<DWORD>(HRESULT_FROM_WIN32(lastError));
                 }
-                // else we call convertUTF8ChunkToUTF16 with an empty string_view to convert possible remaining partials to U+FFFD
             }
 
             const auto result{ til::u8u16(std::string_view{ _buffer.data(), read }, _u16Str, _u8State) };
@@ -710,6 +685,11 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
     winrt::event_token ConptyConnection::NewConnection(const NewConnectionHandler& handler) { return _newConnectionHandlers.add(handler); };
     void ConptyConnection::NewConnection(const winrt::event_token& token) { _newConnectionHandlers.remove(token); };
 
+    void ConptyConnection::closePseudoConsoleAsync(HPCON hPC) noexcept
+    {
+        ::ConptyClosePseudoConsoleTimeout(hPC, 0);
+    }
+
     HRESULT ConptyConnection::NewHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client, TERMINAL_STARTUP_INFO startupInfo) noexcept
     try
     {

src/cascadia/TerminalConnection/ConptyConnection.h

@@ -6,16 +6,8 @@
 #include "ConptyConnection.g.h"
 #include "ConnectionStateHolder.h"
 
-#include <conpty-static.h>
-
 #include "ITerminalHandoff.h"
 
-namespace wil
-{
-    // These belong in WIL upstream, so when we reingest the change that has them we'll get rid of ours.
-    using unique_static_pseudoconsole_handle = wil::unique_any<HPCON, decltype(&::ConptyClosePseudoConsole), ::ConptyClosePseudoConsoleNoWait>;
-}
-
 namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
 {
     struct ConptyConnection : ConptyConnectionT<ConptyConnection>, ConnectionStateHolder<ConptyConnection>
@@ -65,12 +57,13 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
         WINRT_CALLBACK(TerminalOutput, TerminalOutputHandler);
 
     private:
+        static void closePseudoConsoleAsync(HPCON hPC) noexcept;
         static HRESULT NewHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client, TERMINAL_STARTUP_INFO startupInfo) noexcept;
         static winrt::hstring _commandlineFromProcess(HANDLE process);
 
         HRESULT _LaunchAttachedClient() noexcept;
         void _indicateExitWithStatus(unsigned int status) noexcept;
-        void _ClientTerminated() noexcept;
+        void _LastConPtyClientDisconnected() noexcept;
 
         til::CoordType _rows{};
         til::CoordType _cols{};
@@ -90,8 +83,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
         wil::unique_hfile _outPipe; // The pipe for reading output from
         wil::unique_handle _hOutputThread;
         wil::unique_process_information _piClient;
-        wil::unique_static_pseudoconsole_handle _hPC;
-        wil::unique_threadpool_wait _clientExitWait;
+        wil::unique_any<HPCON, decltype(closePseudoConsoleAsync), closePseudoConsoleAsync> _hPC;
 
         til::u8state _u8State{};
         std::wstring _u16Str{};

src/host/PtySignalInputThread.cpp

@@ -350,6 +350,6 @@ void PtySignalInputThread::_DoSetWindowParent(const SetParentData& data)
 // - <none>
 void PtySignalInputThread::_Shutdown()
 {
-    // Trigger process shutdown.
-    CloseConsoleProcessState();
+    auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
+    gci.GetVtIo()->SendCloseEvent();
 }

src/host/VtIo.cpp

@@ -446,7 +446,7 @@ void VtIo::SetWindowVisibility(bool showOrHide) noexcept
 void VtIo::CloseInput()
 {
     _pVtInputThread = nullptr;
-    _shutdownNow();
+    SendCloseEvent();
 }
 
 void VtIo::CloseOutput()
@@ -455,14 +455,18 @@ void VtIo::CloseOutput()
     g.getConsoleInformation().GetActiveOutputBuffer().SetTerminalConnection(nullptr);
 }
 
-void VtIo::_shutdownNow()
+void VtIo::SendCloseEvent()
 {
-    // At this point, we no longer have a renderer or inthread. So we've
-    //      effectively been disconnected from the terminal.
+    LockConsole();
+    const auto unlock = wil::scope_exit([] { UnlockConsole(); });
 
-    // If we have any remaining attached processes, this will prepare us to send a ctrl+close to them
-    // if we don't, this will cause us to rundown and exit.
-    CloseConsoleProcessState();
+    // This function is called when the ConPTY signal pipe is closed (PtySignalInputThread) and when the input
+    // pipe is closed (VtIo). Usually these two happen at about the same time. This if condition is a bit of
+    // a premature optimization and prevents us from sending out a CTRL_CLOSE_EVENT right after another.
+    if (!std::exchange(_closeEventSent, true))
+    {
+        CloseConsoleProcessState();
+    }
 }
 
 // Method Description:

src/host/VtIo.hpp

@@ -32,11 +32,10 @@ namespace Microsoft::Console::VirtualTerminal
         [[nodiscard]] HRESULT StartIfNeeded();
 
         [[nodiscard]] static HRESULT ParseIoMode(const std::wstring& VtMode, _Out_ VtIoMode& ioMode);
-
         [[nodiscard]] HRESULT SuppressResizeRepaint();
         [[nodiscard]] HRESULT SetCursorPosition(const til::point coordCursor);
-
         [[nodiscard]] HRESULT SwitchScreenBuffer(const bool useAltBuffer);
+        void SendCloseEvent();
 
         void CloseInput();
         void CloseOutput();
@@ -71,15 +70,14 @@ namespace Microsoft::Console::VirtualTerminal
         bool _resizeQuirk{ false };
         bool _win32InputMode{ false };
         bool _passthroughMode{ false };
+        bool _closeEventSent{ false };
 
         std::unique_ptr<Microsoft::Console::Render::VtEngine> _pVtRenderEngine;
         std::unique_ptr<Microsoft::Console::VtInputThread> _pVtInputThread;
         std::unique_ptr<Microsoft::Console::PtySignalInputThread> _pPtySignalInputThread;
 
         [[nodiscard]] HRESULT _Initialize(const HANDLE InHandle, const HANDLE OutHandle, const std::wstring& VtMode, _In_opt_ const HANDLE SignalHandle);
 
-        void _shutdownNow();
-
 #ifdef UNIT_TESTING
         friend class VtIoTests;
 #endif

src/host/output.cpp

@@ -499,9 +499,6 @@ void SetActiveScreenBuffer(SCREEN_INFORMATION& screenInfo)
 // TODO: MSFT 9450717 This should join the ProcessList class when CtrlEvents become moved into the server. https://osgvsowi/9450717
 void CloseConsoleProcessState()
 {
-    LockConsole();
-    const auto unlock = wil::scope_exit([] { UnlockConsole(); });
-
     auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
 
     // If there are no connected processes, sending control events is pointless as there's no one do send them to. In

src/inc/conpty-static.h

@@ -28,19 +28,15 @@
 #define PSEUDOCONSOLE_PASSTHROUGH_MODE (8u)
 
 CONPTY_EXPORT HRESULT WINAPI ConptyCreatePseudoConsole(COORD size, HANDLE hInput, HANDLE hOutput, DWORD dwFlags, HPCON* phPC);
-
 CONPTY_EXPORT HRESULT WINAPI ConptyCreatePseudoConsoleAsUser(HANDLE hToken, COORD size, HANDLE hInput, HANDLE hOutput, DWORD dwFlags, HPCON* phPC);
 
 CONPTY_EXPORT HRESULT WINAPI ConptyResizePseudoConsole(HPCON hPC, COORD size);
-
 CONPTY_EXPORT HRESULT WINAPI ConptyClearPseudoConsole(HPCON hPC);
-
 CONPTY_EXPORT HRESULT WINAPI ConptyShowHidePseudoConsole(HPCON hPC, bool show);
-
 CONPTY_EXPORT HRESULT WINAPI ConptyReparentPseudoConsole(HPCON hPC, HWND newParent);
+CONPTY_EXPORT HRESULT WINAPI ConptyReleasePseudoConsole(HPCON hPC);
 
 CONPTY_EXPORT VOID WINAPI ConptyClosePseudoConsole(HPCON hPC);
-
-CONPTY_EXPORT VOID WINAPI ConptyClosePseudoConsoleNoWait(HPCON hPC);
+CONPTY_EXPORT VOID WINAPI ConptyClosePseudoConsoleTimeout(HPCON hPC, DWORD dwMilliseconds);
 
 CONPTY_EXPORT HRESULT WINAPI ConptyPackPseudoConsole(HANDLE hServerProcess, HANDLE hRef, HANDLE hSignal, HPCON* phPC);

src/winconpty/dll/winconpty.def

@@ -4,10 +4,11 @@ EXPORTS
     ConptyCreatePseudoConsoleAsUser
     ConptyResizePseudoConsole
     ConptyClosePseudoConsole
-    ConptyClosePseudoConsoleNoWait
+    ConptyClosePseudoConsoleTimeout
     ConptyClearPseudoConsole
     ConptyShowHidePseudoConsole
     ConptyReparentPseudoConsole
+    ConptyReleasePseudoConsole
     ConptyPackPseudoConsole
 
     ; Compatibility aliases for P/Invoke; only required for compatibility