Begin fixing terminal handoff

src/cascadia/CascadiaPackage/Package-Dev.appxmanifest

@@ -109,8 +109,7 @@
             <com:ComInterface>
                 <com:ProxyStub Id="DEC4804D-56D1-4F73-9FBE-6828E7C85C56" DisplayName="OpenConsoleHandoffProxy" Path="OpenConsoleProxy.dll"/>
                 <com:Interface Id="E686C757-9A35-4A1C-B3CE-0BCC8B5C69F4" ProxyStubClsid="DEC4804D-56D1-4F73-9FBE-6828E7C85C56"/>
-                <com:Interface Id="59D55CCE-FC8A-48B4-ACE8-0A9286C6557F" ProxyStubClsid="DEC4804D-56D1-4F73-9FBE-6828E7C85C56"/> <!-- ITerminalHandoff -->
+                <com:Interface Id="77605BF7-D950-4013-9C37-E959E59B0D9D" ProxyStubClsid="DEC4804D-56D1-4F73-9FBE-6828E7C85C56"/> <!-- ITerminalHandoff3 -->
-                <com:Interface Id="AA6B364F-4A50-4176-9002-0AE755E7B5EF" ProxyStubClsid="DEC4804D-56D1-4F73-9FBE-6828E7C85C56"/> <!-- ITerminalHandoff2 -->
                 <com:Interface Id="746E6BC0-AB05-4E38-AB14-71E86763141F" ProxyStubClsid="DEC4804D-56D1-4F73-9FBE-6828E7C85C56"/>
             </com:ComInterface>
         </com:Extension>

src/cascadia/CascadiaPackage/Package-Pre.appxmanifest

@@ -198,8 +198,7 @@
             <com:ComInterface>
                 <com:ProxyStub Id="1833E661-CC81-4DD0-87C6-C2F74BD39EFA" DisplayName="OpenConsoleHandoffProxy" Path="OpenConsoleProxy.dll"/>
                 <com:Interface Id="E686C757-9A35-4A1C-B3CE-0BCC8B5C69F4" ProxyStubClsid="1833E661-CC81-4DD0-87C6-C2F74BD39EFA"/>
-                <com:Interface Id="59D55CCE-FC8A-48B4-ACE8-0A9286C6557F" ProxyStubClsid="1833E661-CC81-4DD0-87C6-C2F74BD39EFA"/> <!-- ITerminalHandoff -->
+                <com:Interface Id="77605BF7-D950-4013-9C37-E959E59B0D9D" ProxyStubClsid="1833E661-CC81-4DD0-87C6-C2F74BD39EFA"/> <!-- ITerminalHandoff3 -->
-                <com:Interface Id="AA6B364F-4A50-4176-9002-0AE755E7B5EF" ProxyStubClsid="1833E661-CC81-4DD0-87C6-C2F74BD39EFA"/> <!-- ITerminalHandoff2 -->
                 <com:Interface Id="746E6BC0-AB05-4E38-AB14-71E86763141F" ProxyStubClsid="1833E661-CC81-4DD0-87C6-C2F74BD39EFA"/>
             </com:ComInterface>
         </com:Extension>

src/cascadia/CascadiaPackage/Package.appxmanifest

@@ -198,8 +198,7 @@
             <com:ComInterface>
                 <com:ProxyStub Id="3171DE52-6EFA-4AEF-8A9F-D02BD67E7A4F" DisplayName="OpenConsoleHandoffProxy" Path="OpenConsoleProxy.dll"/>
                 <com:Interface Id="E686C757-9A35-4A1C-B3CE-0BCC8B5C69F4" ProxyStubClsid="3171DE52-6EFA-4AEF-8A9F-D02BD67E7A4F"/>
-                <com:Interface Id="59D55CCE-FC8A-48B4-ACE8-0A9286C6557F" ProxyStubClsid="3171DE52-6EFA-4AEF-8A9F-D02BD67E7A4F"/> <!-- ITerminalHandoff -->
+                <com:Interface Id="77605BF7-D950-4013-9C37-E959E59B0D9D" ProxyStubClsid="3171DE52-6EFA-4AEF-8A9F-D02BD67E7A4F"/> <!-- ITerminalHandoff3 -->
-                <com:Interface Id="AA6B364F-4A50-4176-9002-0AE755E7B5EF" ProxyStubClsid="3171DE52-6EFA-4AEF-8A9F-D02BD67E7A4F"/> <!-- ITerminalHandoff2 -->
                 <com:Interface Id="746E6BC0-AB05-4E38-AB14-71E86763141F" ProxyStubClsid="3171DE52-6EFA-4AEF-8A9F-D02BD67E7A4F"/>
             </com:ComInterface>
         </com:Extension>

src/cascadia/TerminalConnection/CTerminalHandoff.cpp

@@ -57,7 +57,7 @@ HRESULT CTerminalHandoff::s_StopListening()
 }
 
 // See s_StopListening()
-HRESULT CTerminalHandoff::s_StopListeningLocked()
+HRESULT CTerminalHandoff::s_StopListeningLocked() noexcept
 {
     RETURN_HR_IF_NULL(E_NOT_VALID_STATE, _pfnHandoff);
 
@@ -72,91 +72,71 @@ HRESULT CTerminalHandoff::s_StopListeningLocked()
     return S_OK;
 }
 
-// Routine Description:
-// - Helper to duplicate a handle to ourselves so we can keep holding onto it
-//   after the caller frees the original one.
-// Arguments:
-// - in - Handle to duplicate
-// - out - Where to place the duplicated value
-// Return Value:
-// - S_OK or Win32 error from `::DuplicateHandle`
-static HRESULT _duplicateHandle(const HANDLE in, HANDLE& out) noexcept
-{
-    RETURN_IF_WIN32_BOOL_FALSE(::DuplicateHandle(GetCurrentProcess(), in, GetCurrentProcess(), &out, 0, FALSE, DUPLICATE_SAME_ACCESS));
-    return S_OK;
-}
-
 // Routine Description:
 // - Receives the terminal handoff via COM from the other process,
 //   duplicates handles as COM will free those given on the way out,
 //   then fires off an event notifying the rest of the terminal that
 //   a connection is on its way in.
 // Arguments:
-// - in - PTY input handle that we will read from
+// - pipes:
-// - out - PTY output handle that we will write to
+//   0: in - PTY input handle that we will read from
-// - signal - PTY signal handle for out of band messaging
+//   1: out - PTY output handle that we will write to
-// - ref - Client reference handle for console session so it stays alive until we let go
+//   2: signal - PTY signal handle for out of band messaging
-// - server - PTY process handle to track for lifetime/cleanup
+// - processes:
-// - client - Process handle to client so we can track its lifetime and exit appropriately
+//   0: server - PTY process handle to track for lifetime/cleanup
+//   1: client - Process handle to client so we can track its lifetime and exit appropriately
 // Return Value:
 // - E_NOT_VALID_STATE if a event handler is not registered before calling. `::DuplicateHandle`
 //   error codes if we cannot manage to make our own copy of handles to retain. Or S_OK/error
 //   from the registered handler event function.
-HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client, TERMINAL_STARTUP_INFO startupInfo)
+HRESULT CTerminalHandoff::EstablishPtyHandoff(const HANDLE* pipes, const HANDLE* processes, TERMINAL_STARTUP_INFO startupInfo, PTY_HANDOFF_RESPONSE* response) noexcept
+try
 {
-    try
+    std::unique_lock lock{ _mtx };
-    {
-        std::unique_lock lock{ _mtx };
-
-        // s_StopListeningLocked sets _pfnHandoff to nullptr.
-        // localPfnHandoff is tested for nullness below.
-#pragma warning(suppress : 26429) // Symbol '...' is never tested for nullness, it can be marked as not_null (f.23).
-        auto localPfnHandoff = _pfnHandoff;
 
+    const auto stopListeningOnExit = wil::scope_exit([]() {
         // Because we are REGCLS_SINGLEUSE... we need to `CoRevokeClassObject` after we handle this ONE call.
         // COM does not automatically clean that up for us. We must do it.
         LOG_IF_FAILED(s_StopListeningLocked());
+    });
 
-        // Report an error if no one registered a handoff function before calling this.
+    // Report an error if no one registered a handoff function before calling this.
-        THROW_HR_IF_NULL(E_NOT_VALID_STATE, localPfnHandoff);
+    THROW_HR_IF_NULL(E_NOT_VALID_STATE, _pfnHandoff);
 
-        // Duplicate the handles from what we received.
+    // If the client didn't specify a response object, that's fine. It'll work anyways and
-        // The contract with COM specifies that any HANDLEs we receive from the caller belong
+    // they just won't get any information from us. To avoid forcing our handoff callback
-        // to the caller and will be freed when we leave the scope of this method.
+    // target to check the pointer, we'll just use a temporary throw-away object here.
-        // Making our own duplicate copy ensures they hang around in our lifetime.
+    PTY_HANDOFF_RESPONSE tempResponse;
-        THROW_IF_FAILED(_duplicateHandle(in, in));
+    if (!response)
-        THROW_IF_FAILED(_duplicateHandle(out, out));
-        THROW_IF_FAILED(_duplicateHandle(signal, signal));
-        THROW_IF_FAILED(_duplicateHandle(ref, ref));
-        THROW_IF_FAILED(_duplicateHandle(server, server));
-        THROW_IF_FAILED(_duplicateHandle(client, client));
-
-        // Call registered handler from when we started listening.
-        THROW_IF_FAILED(localPfnHandoff(in, out, signal, ref, server, client, startupInfo));
-
-#pragma warning(suppress : 26477)
-        TraceLoggingWrite(
-            g_hTerminalConnectionProvider,
-            "ReceiveTerminalHandoff_Success",
-            TraceLoggingDescription("successfully received a terminal handoff"),
-            TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
-            TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage));
-
-        return S_OK;
-    }
-    catch (...)
     {
-        const auto hr = wil::ResultFromCaughtException();
+        response = &tempResponse;
+    }
+
+    // Call registered handler from when we started listening.
+    THROW_IF_FAILED(_pfnHandoff(pipes, processes, startupInfo, *response));
 
 #pragma warning(suppress : 26477)
-        TraceLoggingWrite(
+    TraceLoggingWrite(
-            g_hTerminalConnectionProvider,
+        g_hTerminalConnectionProvider,
-            "ReceiveTerminalHandoff_Failed",
+        "ReceiveTerminalHandoff_Success",
-            TraceLoggingDescription("failed while receiving a terminal handoff"),
+        TraceLoggingDescription("successfully received a terminal handoff"),
-            TraceLoggingHResult(hr),
+        TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
-            TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
+        TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage));
-            TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage));
 
-        return hr;
+    return S_OK;
-    }
+}
+catch (...)
+{
+    const auto hr = wil::ResultFromCaughtException();
+
+#pragma warning(suppress : 26477)
+    TraceLoggingWrite(
+        g_hTerminalConnectionProvider,
+        "ReceiveTerminalHandoff_Failed",
+        TraceLoggingDescription("failed while receiving a terminal handoff"),
+        TraceLoggingHResult(hr),
+        TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
+        TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage));
+
+    return hr;
 }

src/cascadia/TerminalConnection/CTerminalHandoff.h

@@ -26,27 +26,22 @@ Author(s):
 #define __CLSID_CTerminalHandoff "051F34EE-C1FD-4B19-AF75-9BA54648434C"
 #endif
 
-using NewHandoffFunction = HRESULT (*)(HANDLE, HANDLE, HANDLE, HANDLE, HANDLE, HANDLE, TERMINAL_STARTUP_INFO);
+using NewHandoffFunction = HRESULT (*)(const HANDLE* pipes, const HANDLE* processes, const TERMINAL_STARTUP_INFO& startupInfo, PTY_HANDOFF_RESPONSE& response);
 
 struct __declspec(uuid(__CLSID_CTerminalHandoff))
-    CTerminalHandoff : public Microsoft::WRL::RuntimeClass<Microsoft::WRL::RuntimeClassFlags<Microsoft::WRL::RuntimeClassType::ClassicCom>, ITerminalHandoff2>
+    CTerminalHandoff : public Microsoft::WRL::RuntimeClass<Microsoft::WRL::RuntimeClassFlags<Microsoft::WRL::RuntimeClassType::ClassicCom>, ITerminalHandoff3>
 {
-#pragma region ITerminalHandoff
+    STDMETHODIMP EstablishPtyHandoff(
-    STDMETHODIMP EstablishPtyHandoff(HANDLE in,
+        /* [size_is][system_handle][in] */ const HANDLE* pipes,
-                                     HANDLE out,
+        /* [size_is][system_handle][in] */ const HANDLE* processes,
-                                     HANDLE signal,
+        /* [in] */ TERMINAL_STARTUP_INFO startupInfo,
-                                     HANDLE ref,
+        /* [out] */ PTY_HANDOFF_RESPONSE* response) noexcept override;
-                                     HANDLE server,
-                                     HANDLE client,
-                                     TERMINAL_STARTUP_INFO startupInfo) override;
-
-#pragma endregion
 
     static HRESULT s_StartListening(NewHandoffFunction pfnHandoff);
     static HRESULT s_StopListening();
 
 private:
-    static HRESULT s_StopListeningLocked();
+    static HRESULT s_StopListeningLocked() noexcept;
 };
 
 // Disable warnings from the CoCreatableClass macro as the value it provides for

src/cascadia/TerminalConnection/ConptyConnection.cpp

@@ -34,6 +34,14 @@ static constexpr auto _errorFormat = L"{0} ({0:#010x})"sv;
 
 namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
 {
+    static HANDLE duplicateHandle(const HANDLE in)
+    {
+        HANDLE out;
+        const auto currentProcess = GetCurrentProcess();
+        THROW_IF_WIN32_BOOL_FALSE(::DuplicateHandle(currentProcess, in, currentProcess, &out, 0, FALSE, DUPLICATE_SAME_ACCESS));
+        return out;
+    }
+
     // Function Description:
     // - creates some basic anonymous pipes and passes them to CreatePseudoConsole
     // Arguments:
@@ -202,30 +210,24 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
     }
     CATCH_RETURN();
 
-    ConptyConnection::ConptyConnection(const HANDLE hSig,
+    ConptyConnection::ConptyConnection(const HANDLE* pipes, const HANDLE* processes, const TERMINAL_STARTUP_INFO& startupInfo) :
-                                       const HANDLE hIn,
-                                       const HANDLE hOut,
-                                       const HANDLE hRef,
-                                       const HANDLE hServerProcess,
-                                       const HANDLE hClientProcess,
-                                       TERMINAL_STARTUP_INFO startupInfo) :
         _rows{ 25 },
         _cols{ 80 },
         _guid{ Utils::CreateGuid() },
-        _inPipe{ hIn },
+        _inPipe{ duplicateHandle(pipes[0]) },
-        _outPipe{ hOut }
+        _outPipe{ duplicateHandle(pipes[1]) }
     {
-        THROW_IF_FAILED(ConptyPackPseudoConsole(hServerProcess, hRef, hSig, &_hPC));
+        THROW_IF_FAILED(ConptyPackPseudoConsole(duplicateHandle(processes[0]), nullptr, duplicateHandle(pipes[2]), &_hPC));
-        _piClient.hProcess = hClientProcess;
+        _piClient.hProcess = duplicateHandle(processes[1]);
 
-        _startupInfo.title = winrt::hstring{ startupInfo.pszTitle, SysStringLen(startupInfo.pszTitle) };
+        _startupInfo.title = winrt::hstring{ startupInfo.pbTitle, SysStringLen(startupInfo.pbTitle) };
-        _startupInfo.iconPath = winrt::hstring{ startupInfo.pszIconPath, SysStringLen(startupInfo.pszIconPath) };
+        _startupInfo.iconPath = winrt::hstring{ startupInfo.pbIconPath, SysStringLen(startupInfo.pbIconPath) };
         _startupInfo.iconIndex = startupInfo.iconIndex;
         _startupInfo.showWindow = startupInfo.wShowWindow;
 
         try
         {
-            _commandline = _commandlineFromProcess(hClientProcess);
+            _commandline = _commandlineFromProcess(_piClient.hProcess);
         }
         CATCH_LOG()
     }
@@ -703,10 +705,11 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
         ::ConptyClosePseudoConsoleTimeout(hPC, 0);
     }
 
-    HRESULT ConptyConnection::NewHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client, TERMINAL_STARTUP_INFO startupInfo) noexcept
+    HRESULT ConptyConnection::NewHandoff(const HANDLE* pipes, const HANDLE* processes, const TERMINAL_STARTUP_INFO& startupInfo, PTY_HANDOFF_RESPONSE& response) noexcept
     try
     {
-        _newConnectionHandlers(winrt::make<ConptyConnection>(signal, in, out, ref, server, client, startupInfo));
+        response = {};
+        _newConnectionHandlers(winrt::make<ConptyConnection>(pipes, processes, startupInfo));
 
         return S_OK;
     }

src/cascadia/TerminalConnection/ConptyConnection.h

@@ -12,15 +12,9 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
 {
     struct ConptyConnection : ConptyConnectionT<ConptyConnection>, ConnectionStateHolder<ConptyConnection>
     {
-        ConptyConnection(const HANDLE hSig,
-                         const HANDLE hIn,
-                         const HANDLE hOut,
-                         const HANDLE hRef,
-                         const HANDLE hServerProcess,
-                         const HANDLE hClientProcess,
-                         TERMINAL_STARTUP_INFO startupInfo);
-
         ConptyConnection() noexcept = default;
+        ConptyConnection(const HANDLE* pipes, const HANDLE* processes, const TERMINAL_STARTUP_INFO& startupInfo);
+
         void Initialize(const Windows::Foundation::Collections::ValueSet& settings);
 
         static winrt::fire_and_forget final_release(std::unique_ptr<ConptyConnection> connection);
@@ -59,7 +53,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
 
     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 HRESULT NewHandoff(const HANDLE* pipes, const HANDLE* processes, const TERMINAL_STARTUP_INFO& startupInfo, PTY_HANDOFF_RESPONSE& response) noexcept;
         static winrt::hstring _commandlineFromProcess(HANDLE process);
 
         HRESULT _LaunchAttachedClient() noexcept;

src/host/proxy/Host.Proxy.vcxproj

@@ -38,11 +38,13 @@
       <HeaderFileName>IConsoleHandoff.h</HeaderFileName>
       <MinimumTargetSystem>NT100</MinimumTargetSystem>
       <OutputDirectory>$(IntDir)</OutputDirectory>
+      <WarningLevel>3</WarningLevel>
     </Midl>
     <Midl Include="ITerminalHandoff.idl">
       <HeaderFileName>ITerminalHandoff.h</HeaderFileName>
       <MinimumTargetSystem>NT100</MinimumTargetSystem>
       <OutputDirectory>$(IntDir)</OutputDirectory>
+      <WarningLevel>3</WarningLevel>
     </Midl>
   </ItemGroup>
   <ItemGroup>

src/host/proxy/IConsoleHandoff.idl

@@ -1,8 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import "oaidl.idl";
+import "unknwn.idl";
-import "ocidl.idl";
 
 typedef struct _CONSOLE_PORTABLE_ATTACH_MSG
 {

src/host/proxy/ITerminalHandoff.idl

@@ -1,17 +1,15 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import "oaidl.idl";
+import "unknwn.idl";
-import "ocidl.idl";
-
 
 typedef struct _TERMINAL_STARTUP_INFO
 {
     // In STARTUPINFO
-    BSTR pszTitle;
+    BSTR pbTitle;
 
     // Also wanted
-    BSTR pszIconPath;
+    BSTR pbIconPath;
     LONG iconIndex;
 
     // The rest of STARTUPINFO
@@ -26,6 +24,19 @@ typedef struct _TERMINAL_STARTUP_INFO
     WORD wShowWindow;
 } TERMINAL_STARTUP_INFO;
 
+// All of the following members are optional. When migrating from ITerminalHandoff2
+// or earlier, you _can_ choose to return an all-0 struct. It will work.
+typedef struct _PTY_HANDOFF_RESPONSE
+{
+    // Corresponds to ConptyResizePseudoConsole
+    DWORD dwXSize;
+    DWORD dwYSize;
+    // Corresponds to ConptyReparentPseudoConsole
+    HWND hParentWindow;
+    // Corresponds to ConptyShowHidePseudoConsole
+    BOOL bShow;
+} PTY_HANDOFF_RESPONSE;
+
 // LOAD BEARING!
 //
 // There is only ever one OpenConsoleProxy.dll loaded by COM for _ALL_ terminal
@@ -33,6 +44,7 @@ typedef struct _TERMINAL_STARTUP_INFO
 // versions of interfaces in the file here, even if the old version is no longer
 // in use.
 
+// The original prototype interface for handoff
 [
     object,
     uuid(59D55CCE-FC8A-48B4-ACE8-0A9286C6557F)
@@ -47,11 +59,15 @@ typedef struct _TERMINAL_STARTUP_INFO
                                 [in, system_handle(sh_process)] HANDLE client);
 };
 
+// This variant was added in v1.15.2874 when we noticed that we need the STARTUPINFO information to properly handle
+// .lnk files. It was previously assumed that this wouldn't be necessary, because we could just get the path out of
+// the PEB, but then noticed that this won't work, because the PEB contains the .exe path. Hindsight is 20/20.
 [
     object,
     uuid(AA6B364F-4A50-4176-9002-0AE755E7B5EF)
 ] interface ITerminalHandoff2 : IUnknown
 {
+    // DEPRECATED!
     HRESULT EstablishPtyHandoff([in, system_handle(sh_pipe)] HANDLE in,
                                 [in, system_handle(sh_pipe)] HANDLE out,
                                 [in, system_handle(sh_pipe)] HANDLE signal,
@@ -60,3 +76,26 @@ typedef struct _TERMINAL_STARTUP_INFO
                                 [in, system_handle(sh_process)] HANDLE client,
                                 [in] TERMINAL_STARTUP_INFO startupInfo);
 };
+
+// This variant was added in $FUTURE_VER when we noticed that there's a race condition between OpenConsole
+// starting and the terminal calling ConptyResizePseudoConsole. If by that time the client application has
+// already called GetConsoleScreenBufferInfo to get the window size, it'll now assume the wrong size.
+//
+// It also drops the "ref" parameter, because we found that holding onto it is quite error prone.
+// Any reasonable terminal should call ConptyReleasePseudoConsole as soon as they can.
+//
+// Finally, it starts passing handles as arrays, because 7 parameters are a tad bit too much. I would've
+// preferred a "PTY_HANDOFF_REQUEST" struct with named parameters, but coming from the land of the free,
+// COM choose to remain severely restricted when it comes to what you can put into a struct. Why though.
+[
+    object,
+    uuid(77605BF7-D950-4013-9C37-E959E59B0D9D)
+]
+interface ITerminalHandoff3 : IUnknown
+{
+    HRESULT EstablishPtyHandoff(
+        [in, system_handle(sh_pipe), size_is(3)] const HANDLE* pipes,
+        [in, system_handle(sh_process), size_is(2)] const HANDLE* processes,
+        [in] TERMINAL_STARTUP_INFO startupInfo,
+        [out] PTY_HANDOFF_RESPONSE* response);
+};

src/host/srvinit.cpp

@@ -475,20 +475,13 @@ try
     std::unique_ptr<IConsoleControl> remoteControl = std::make_unique<Microsoft::Console::Interactivity::RemoteConsoleControl>(hostSignalPipe);
     RETURN_IF_NTSTATUS_FAILED(ServiceLocator::SetConsoleControlInstance(std::move(remoteControl)));
 
-    wil::unique_handle signalPipeTheirSide;
+    wil::unique_handle pipesOurSide[3];
-    wil::unique_handle signalPipeOurSide;
+    wil::unique_handle pipesTheirSide[3];
 
-    wil::unique_handle inPipeTheirSide;
+    for (size_t i = 0; i < 3; ++i)
-    wil::unique_handle inPipeOurSide;
+    {
-
+        RETURN_IF_WIN32_BOOL_FALSE(CreatePipe(pipesOurSide[i].addressof(), pipesTheirSide[i].addressof(), nullptr, 0));
-    wil::unique_handle outPipeTheirSide;
+    }
-    wil::unique_handle outPipeOurSide;
-
-    RETURN_IF_WIN32_BOOL_FALSE(CreatePipe(signalPipeOurSide.addressof(), signalPipeTheirSide.addressof(), nullptr, 0));
-
-    RETURN_IF_WIN32_BOOL_FALSE(CreatePipe(inPipeOurSide.addressof(), inPipeTheirSide.addressof(), nullptr, 0));
-
-    RETURN_IF_WIN32_BOOL_FALSE(CreatePipe(outPipeTheirSide.addressof(), outPipeOurSide.addressof(), nullptr, 0));
 
     TraceLoggingWrite(g_hConhostV2EventTraceProvider,
                       "SrvInit_ReceiveHandoff_OpenedPipes",
@@ -503,29 +496,16 @@ try
                       TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
                       TraceLoggingKeyword(TIL_KEYWORD_TRACE));
 
+    // TODO: don't create this
     wil::unique_handle refHandle;
     RETURN_IF_NTSTATUS_FAILED(DeviceHandle::CreateClientHandle(refHandle.addressof(),
                                                                Server,
                                                                L"\\Reference",
                                                                FALSE));
+    //refHandle.release();
 
     const auto serverProcess = GetCurrentProcess();
 
-    ::Microsoft::WRL::ComPtr<ITerminalHandoff2> handoff;
-
-    TraceLoggingWrite(g_hConhostV2EventTraceProvider,
-                      "SrvInit_PrepareToCreateDelegationTerminal",
-                      TraceLoggingGuid(g.delegationPair.terminal, "TerminalClsid"),
-                      TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
-                      TraceLoggingKeyword(TIL_KEYWORD_TRACE));
-
-    RETURN_IF_FAILED(CoCreateInstance(g.delegationPair.terminal, nullptr, CLSCTX_LOCAL_SERVER, IID_PPV_ARGS(&handoff)));
-    TraceLoggingWrite(g_hConhostV2EventTraceProvider,
-                      "SrvInit_CreatedDelegationTerminal",
-                      TraceLoggingGuid(g.delegationPair.terminal, "TerminalClsid"),
-                      TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
-                      TraceLoggingKeyword(TIL_KEYWORD_TRACE));
-
     // As a part of defterm handoff, we're gonna try to pull a lot of
     // information out of the link and startup info, so we can let the terminal
     // know these things as well.
@@ -586,34 +566,40 @@ try
 
     myStartupInfo.wShowWindow = settings.GetShowWindow();
 
-    RETURN_IF_FAILED(handoff->EstablishPtyHandoff(inPipeTheirSide.get(),
+    // GH#13211 - Make sure we request win32input mode and that the terminal
-                                                  outPipeTheirSide.get(),
+    // obeys the resizing quirk. Otherwise, defterm connections to the Terminal
-                                                  signalPipeTheirSide.get(),
+    // are going to have weird resizing, and aren't going to send full fidelity
-                                                  refHandle.get(),
+    // input messages.
-                                                  serverProcess,
+    const auto commandLine = fmt::format(FMT_COMPILE(L" --headless --resizeQuirk --win32input --signal {:#x}"), reinterpret_cast<int64_t>(pipesOurSide[2].release()));
-                                                  clientProcess.get(),
+    ConsoleArguments consoleArgs(commandLine, pipesOurSide[0].release(), pipesOurSide[1].release());
-                                                  myStartupInfo));
+    RETURN_IF_FAILED(consoleArgs.ParseCommandline());
+    RETURN_IF_FAILED(ConsoleCreateIoThread(Server, &consoleArgs, driverInputEvent, connectMessage));
+
+    TraceLoggingWrite(g_hConhostV2EventTraceProvider,
+                      "SrvInit_PrepareToCreateDelegationTerminal",
+                      TraceLoggingGuid(g.delegationPair.terminal, "TerminalClsid"),
+                      TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
+                      TraceLoggingKeyword(TIL_KEYWORD_TRACE));
+
+    ::Microsoft::WRL::ComPtr<ITerminalHandoff3> handoff;
+    RETURN_IF_FAILED(CoCreateInstance(g.delegationPair.terminal, nullptr, CLSCTX_LOCAL_SERVER, IID_PPV_ARGS(&handoff)));
+
+    TraceLoggingWrite(g_hConhostV2EventTraceProvider,
+                      "SrvInit_CreatedDelegationTerminal",
+                      TraceLoggingGuid(g.delegationPair.terminal, "TerminalClsid"),
+                      TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
+                      TraceLoggingKeyword(TIL_KEYWORD_TRACE));
+
+    const HANDLE processes[2]{ serverProcess, clientProcess.get() };
+    PTY_HANDOFF_RESPONSE response{};
+    RETURN_IF_FAILED(handoff->EstablishPtyHandoff(pipesTheirSide[0].addressof(), &processes[0], myStartupInfo, &response));
 
     TraceLoggingWrite(g_hConhostV2EventTraceProvider,
                       "SrvInit_DelegateToTerminalSucceeded",
                       TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
                       TraceLoggingKeyword(TIL_KEYWORD_TRACE));
 
-    inPipeTheirSide.reset();
+    return S_OK;
-    outPipeTheirSide.reset();
-    signalPipeTheirSide.reset();
-
-    // GH#13211 - Make sure we request win32input mode and that the terminal
-    // obeys the resizing quirk. Otherwise, defterm connections to the Terminal
-    // are going to have weird resizing, and aren't going to send full fidelity
-    // input messages.
-    const auto commandLine = fmt::format(FMT_COMPILE(L" --headless --resizeQuirk --win32input --signal {:#x}"),
-                                         (int64_t)signalPipeOurSide.release());
-
-    ConsoleArguments consoleArgs(commandLine, inPipeOurSide.release(), outPipeOurSide.release());
-    RETURN_IF_FAILED(consoleArgs.ParseCommandline());
-
-    return ConsoleCreateIoThread(Server, &consoleArgs, driverInputEvent, connectMessage);
 #endif // TIL_FEATURE_RECEIVEINCOMINGHANDOFF_ENABLED
 }
 CATCH_RETURN()

src/inc/til/spsc.h

@@ -409,7 +409,8 @@ namespace til::spsc
     template<typename T>
     struct producer
     {
-        explicit producer(details::arc<T>* arc) noexcept :
+        producer() = default;
+        explicit constexpr producer(details::arc<T>* arc) noexcept :
             _arc(arc) {}
 
         producer<T>(const producer<T>&) = delete;
@@ -433,6 +434,15 @@ namespace til::spsc
             drop();
         }
 
+        void drop()
+        {
+            if (_arc)
+            {
+                _arc->drop_producer();
+                _arc = nullptr;
+            }
+        }
+
         // emplace constructs an item in-place at the end of the queue.
         // It returns true, if the item was successfully placed within the queue.
         // The return value will be false, if the consumer is gone.
@@ -514,21 +524,14 @@ namespace til::spsc
         }
 
     private:
-        void drop()
-        {
-            if (_arc)
-            {
-                _arc->drop_producer();
-            }
-        }
-
         details::arc<T>* _arc = nullptr;
     };
 
     template<typename T>
     struct consumer
     {
-        explicit consumer(details::arc<T>* arc) noexcept :
+        consumer() = default;
+        explicit constexpr consumer(details::arc<T>* arc) noexcept :
             _arc(arc) {}
 
         consumer<T>(const consumer<T>&) = delete;
@@ -552,6 +555,15 @@ namespace til::spsc
             drop();
         }
 
+        void drop()
+        {
+            if (_arc)
+            {
+                _arc->drop_consumer();
+                _arc = nullptr;
+            }
+        }
+
         // pop returns the next item in the queue, or std::nullopt if the producer is gone.
         std::optional<T> pop() const
         {
@@ -618,14 +630,6 @@ namespace til::spsc
         }
 
     private:
-        void drop()
-        {
-            if (_arc)
-            {
-                _arc->drop_consumer();
-            }
-        }
-
         details::arc<T>* _arc = nullptr;
     };
 

src/renderer/vt/state.cpp

@@ -46,7 +46,6 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe,
     _circled(false),
     _firstPaint(true),
     _skipCursor(false),
-    _exitResult{ S_OK },
     _terminalOwner{ nullptr },
     _newBottomLine{ false },
     _deferredCursorPos{ INVALID_COORDS },
@@ -65,6 +64,40 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe,
     // member is only defined when UNIT_TESTING is.
     _usingTestCallback = false;
 #endif
+
+    auto [producer, consumer] = til::spsc::channel<char>(4096);
+    _writer = std::move(producer);
+    _writerThread = std::thread([this, consumer = std::move(consumer)]() {
+        char buffer[4096];
+
+        for (;;)
+        {
+            auto [read, alive] = consumer.pop_n(til::spsc::block_initially, &buffer[0], 4096);
+            //display("WriteFile", { &buffer[0], read });
+            const auto fSuccess = WriteFile(_hFile.get(), &buffer[0], gsl::narrow_cast<DWORD>(read), nullptr, nullptr);
+
+            if (!fSuccess)
+            {
+                _hFile.reset();
+                alive = false;
+                if (_terminalOwner)
+                {
+                    _terminalOwner->CloseOutput();
+                }
+            }
+
+            if (!alive)
+            {
+                break;
+            }
+        }
+    });
+}
+
+VtEngine::~VtEngine()
+{
+    _writer.drop();
+    _writerThread.join();
 }
 
 // Method Description:
@@ -138,22 +171,9 @@ CATCH_RETURN();
 
 [[nodiscard]] HRESULT VtEngine::_Flush() noexcept
 {
-    if (_hFile)
+    //display("_Flush", _buffer);
-    {
+    _writer.push_n(til::spsc::block_forever, _buffer.data(), _buffer.size());
-        auto fSuccess = !!WriteFile(_hFile.get(), _buffer.data(), gsl::narrow_cast<DWORD>(_buffer.size()), nullptr, nullptr);
+    _buffer.clear();
-        _buffer.clear();
-        if (!fSuccess)
-        {
-            _exitResult = HRESULT_FROM_WIN32(GetLastError());
-            _hFile.reset();
-            if (_terminalOwner)
-            {
-                _terminalOwner->CloseOutput();
-            }
-            return _exitResult;
-        }
-    }
-
     return S_OK;
 }
 

src/renderer/vt/vtrenderer.hpp

@@ -18,8 +18,8 @@ Author(s):
 #include "../inc/RenderEngineBase.hpp"
 #include "../../types/inc/Viewport.hpp"
 #include "tracing.hpp"
-#include <string>
+
-#include <functional>
+#include <til/spsc.h>
 
 // fwdecl unittest classes
 #ifdef UNIT_TESTING
@@ -46,6 +46,7 @@ namespace Microsoft::Console::Render
 
         VtEngine(_In_ wil::unique_hfile hPipe,
                  const Microsoft::Console::Types::Viewport initialViewport);
+        ~VtEngine() override;
 
         // IRenderEngine
         [[nodiscard]] HRESULT StartPaint() noexcept override;
@@ -95,6 +96,9 @@ namespace Microsoft::Console::Render
         wil::unique_hfile _hFile;
         std::string _buffer;
 
+        til::spsc::producer<char> _writer;
+        std::thread _writerThread;
+
         std::string _formatBuffer;
         std::string _conversionBuffer;
 
@@ -127,7 +131,6 @@ namespace Microsoft::Console::Render
         bool _newBottomLine;
         til::point _deferredCursorPos;
 
-        HRESULT _exitResult;
         Microsoft::Console::VirtualTerminal::VtIo* _terminalOwner;
 
         Microsoft::Console::VirtualTerminal::RenderTracing _trace;

src/winconpty/winconpty.cpp

@@ -589,7 +589,7 @@ extern "C" HRESULT WINAPI ConptyPackPseudoConsole(_In_ HANDLE hProcess,
     RETURN_HR_IF(E_INVALIDARG, nullptr == phPC);
     *phPC = nullptr;
     RETURN_HR_IF(E_INVALIDARG, !_HandleIsValid(hProcess));
-    RETURN_HR_IF(E_INVALIDARG, !_HandleIsValid(hRef));
+    RETURN_HR_IF(E_INVALIDARG, hRef == INVALID_HANDLE_VALUE);
     RETURN_HR_IF(E_INVALIDARG, !_HandleIsValid(hSignal));
 
     auto pPty = (PseudoConsole*)HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(PseudoConsole));