From 7fd9c5c789dc2eae46f74fdd68093da8a97507d8 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Wed, 14 Aug 2024 22:15:50 +0200 Subject: [PATCH] Align the OSS ConPTY API with Windows 11 24H2 (#17704) --- .../TerminalConnection/ConptyConnection.cpp | 2 +- src/inc/conpty-static.h | 1 - src/winconpty/dll/winconpty.def | 2 +- src/winconpty/ft_pty/ConPtyTests.cpp | 17 ++++----- src/winconpty/winconpty.cpp | 35 +++---------------- src/winconpty/winconpty.h | 2 +- 6 files changed, 17 insertions(+), 42 deletions(-) diff --git a/src/cascadia/TerminalConnection/ConptyConnection.cpp b/src/cascadia/TerminalConnection/ConptyConnection.cpp index 1cb7689c4c1..de5fb8c8be4 100644 --- a/src/cascadia/TerminalConnection/ConptyConnection.cpp +++ b/src/cascadia/TerminalConnection/ConptyConnection.cpp @@ -757,7 +757,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation void ConptyConnection::closePseudoConsoleAsync(HPCON hPC) noexcept { - ::ConptyClosePseudoConsoleTimeout(hPC, 0); + ::ConptyClosePseudoConsole(hPC); } HRESULT ConptyConnection::NewHandoff(HANDLE* in, HANDLE* out, HANDLE signal, HANDLE reference, HANDLE server, HANDLE client, const TERMINAL_STARTUP_INFO* startupInfo) noexcept diff --git a/src/inc/conpty-static.h b/src/inc/conpty-static.h index 875470b1575..aa31c67fa3d 100644 --- a/src/inc/conpty-static.h +++ b/src/inc/conpty-static.h @@ -44,6 +44,5 @@ CONPTY_EXPORT HRESULT WINAPI ConptyReparentPseudoConsole(HPCON hPC, HWND newPare CONPTY_EXPORT HRESULT WINAPI ConptyReleasePseudoConsole(HPCON hPC); CONPTY_EXPORT VOID WINAPI ConptyClosePseudoConsole(HPCON hPC); -CONPTY_EXPORT VOID WINAPI ConptyClosePseudoConsoleTimeout(HPCON hPC, DWORD dwMilliseconds); CONPTY_EXPORT HRESULT WINAPI ConptyPackPseudoConsole(HANDLE hServerProcess, HANDLE hRef, HANDLE hSignal, HPCON* phPC); diff --git a/src/winconpty/dll/winconpty.def b/src/winconpty/dll/winconpty.def index ba5d52515f5..9ccbfc14537 100644 --- a/src/winconpty/dll/winconpty.def +++ b/src/winconpty/dll/winconpty.def @@ -4,7 +4,6 @@ EXPORTS ConptyCreatePseudoConsoleAsUser ConptyResizePseudoConsole ConptyClosePseudoConsole - ConptyClosePseudoConsoleTimeout ConptyClearPseudoConsole ConptyShowHidePseudoConsole ConptyReparentPseudoConsole @@ -18,3 +17,4 @@ EXPORTS ResizePseudoConsole = ConptyResizePseudoConsole ClosePseudoConsole = ConptyClosePseudoConsole ClearPseudoConsole = ConptyClearPseudoConsole + ReleasePseudoConsole = ConptyReleasePseudoConsole diff --git a/src/winconpty/ft_pty/ConPtyTests.cpp b/src/winconpty/ft_pty/ConPtyTests.cpp index 699e2c665de..27cfdecf837 100644 --- a/src/winconpty/ft_pty/ConPtyTests.cpp +++ b/src/winconpty/ft_pty/ConPtyTests.cpp @@ -165,10 +165,10 @@ void ConPtyTests::CreateConPtyNoPipes() VERIFY_FAILED(_CreatePseudoConsole(defaultSize, nullptr, nullptr, 0, &pcon)); VERIFY_SUCCEEDED(_CreatePseudoConsole(defaultSize, nullptr, goodOut, 0, &pcon)); - _ClosePseudoConsoleMembers(&pcon, INFINITE); + _ClosePseudoConsoleMembers(&pcon); VERIFY_SUCCEEDED(_CreatePseudoConsole(defaultSize, goodIn, nullptr, 0, &pcon)); - _ClosePseudoConsoleMembers(&pcon, INFINITE); + _ClosePseudoConsoleMembers(&pcon); } void ConPtyTests::CreateConPtyBadSize() @@ -212,7 +212,7 @@ void ConPtyTests::GoodCreate() &pcon)); auto closePty = wil::scope_exit([&] { - _ClosePseudoConsoleMembers(&pcon, INFINITE); + _ClosePseudoConsoleMembers(&pcon); }); } @@ -241,7 +241,7 @@ void ConPtyTests::GoodCreateMultiple() 0, &pcon1)); auto closePty1 = wil::scope_exit([&] { - _ClosePseudoConsoleMembers(&pcon1, INFINITE); + _ClosePseudoConsoleMembers(&pcon1); }); VERIFY_SUCCEEDED( @@ -251,7 +251,7 @@ void ConPtyTests::GoodCreateMultiple() 0, &pcon2)); auto closePty2 = wil::scope_exit([&] { - _ClosePseudoConsoleMembers(&pcon2, INFINITE); + _ClosePseudoConsoleMembers(&pcon2); }); } @@ -278,7 +278,7 @@ void ConPtyTests::SurvivesOnBreakOutput() 0, &pty)); auto closePty1 = wil::scope_exit([&] { - _ClosePseudoConsoleMembers(&pty, INFINITE); + _ClosePseudoConsoleMembers(&pty); }); DWORD dwExit; @@ -337,7 +337,7 @@ void ConPtyTests::DiesOnClose() 0, &pty)); auto closePty1 = wil::scope_exit([&] { - _ClosePseudoConsoleMembers(&pty, INFINITE); + _ClosePseudoConsoleMembers(&pty); }); DWORD dwExit; @@ -361,8 +361,9 @@ void ConPtyTests::DiesOnClose() Log::Comment(NoThrowString().Format(L"Sleep a bit to let the process attach")); Sleep(100); - _ClosePseudoConsoleMembers(&pty, INFINITE); + _ClosePseudoConsoleMembers(&pty); + WaitForSingleObject(hConPtyProcess.get(), 3000); GetExitCodeProcess(hConPtyProcess.get(), &dwExit); VERIFY_ARE_NOT_EQUAL(dwExit, (DWORD)STILL_ACTIVE); } diff --git a/src/winconpty/winconpty.cpp b/src/winconpty/winconpty.cpp index 441d4d2639f..60c6c442f2a 100644 --- a/src/winconpty/winconpty.cpp +++ b/src/winconpty/winconpty.cpp @@ -368,35 +368,22 @@ HRESULT _ReparentPseudoConsole(_In_ const PseudoConsole* const pPty, _In_ const // - wait: If true, waits for conhost/OpenConsole to exit first. // Return Value: // - -void _ClosePseudoConsoleMembers(_In_ PseudoConsole* pPty, _In_ DWORD dwMilliseconds) +void _ClosePseudoConsoleMembers(_In_ PseudoConsole* pPty) { if (pPty != nullptr) { - // See MSFT:19918626 - // First break the signal pipe - this will trigger conhost to tear itself down if (_HandleIsValid(pPty->hSignal)) { CloseHandle(pPty->hSignal); pPty->hSignal = nullptr; } - // The reference handle ensures that conhost keeps running unless ClosePseudoConsole is called. - // We have to call it before calling WaitForSingleObject however in order to not deadlock, - // Due to conhost waiting for all clients to disconnect, while we wait for conhost to exit. if (_HandleIsValid(pPty->hPtyReference)) { CloseHandle(pPty->hPtyReference); pPty->hPtyReference = nullptr; } - // Then, wait on the conhost process before killing it. - // We do this to make sure the conhost finishes flushing any output it - // has yet to send before we hard kill it. if (_HandleIsValid(pPty->hConPtyProcess)) { - if (dwMilliseconds) - { - WaitForSingleObject(pPty->hConPtyProcess, dwMilliseconds); - } - CloseHandle(pPty->hConPtyProcess); pPty->hConPtyProcess = nullptr; } @@ -412,11 +399,11 @@ void _ClosePseudoConsoleMembers(_In_ PseudoConsole* pPty, _In_ DWORD dwMilliseco // - wait: If true, waits for conhost/OpenConsole to exit first. // Return Value: // - -static void _ClosePseudoConsole(_In_ PseudoConsole* pPty, _In_ DWORD dwMilliseconds) noexcept +static void _ClosePseudoConsole(_In_ PseudoConsole* pPty) noexcept { if (pPty != nullptr) { - _ClosePseudoConsoleMembers(pPty, dwMilliseconds); + _ClosePseudoConsoleMembers(pPty); HeapFree(GetProcessHeap(), 0, pPty); } } @@ -477,7 +464,7 @@ extern "C" HRESULT WINAPI ConptyCreatePseudoConsoleAsUser(_In_ HANDLE hToken, auto pPty = (PseudoConsole*)HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(PseudoConsole)); RETURN_IF_NULL_ALLOC(pPty); auto cleanupPty = wil::scope_exit([&]() noexcept { - _ClosePseudoConsole(pPty, 0); + _ClosePseudoConsole(pPty); }); wil::unique_handle duplicatedInput; @@ -574,19 +561,7 @@ extern "C" HRESULT WINAPI ConptyReleasePseudoConsole(_In_ HPCON hPC) // Waits for conhost/OpenConsole to exit first. extern "C" VOID WINAPI ConptyClosePseudoConsole(_In_ HPCON hPC) { - _ClosePseudoConsole((PseudoConsole*)hPC, INFINITE); -} - -// Function Description: -// Closes the conpty and all associated state. -// Client applications attached to the conpty will also behave as though the -// console window they were running in was closed. -// This can fail if the conhost hosting the pseudoconsole failed to be -// terminated, or if the pseudoconsole was already terminated. -// Doesn't wait for conhost/OpenConsole to exit. -extern "C" VOID WINAPI ConptyClosePseudoConsoleTimeout(_In_ HPCON hPC, _In_ DWORD dwMilliseconds) -{ - _ClosePseudoConsole((PseudoConsole*)hPC, dwMilliseconds); + _ClosePseudoConsole((PseudoConsole*)hPC); } // NOTE: This one is not defined in the Windows headers but is diff --git a/src/winconpty/winconpty.h b/src/winconpty/winconpty.h index 11503a2494b..4bb43ddf8f2 100644 --- a/src/winconpty/winconpty.h +++ b/src/winconpty/winconpty.h @@ -71,7 +71,7 @@ HRESULT _ResizePseudoConsole(_In_ const PseudoConsole* const pPty, _In_ const CO HRESULT _ClearPseudoConsole(_In_ const PseudoConsole* const pPty); HRESULT _ShowHidePseudoConsole(_In_ const PseudoConsole* const pPty, const bool show); HRESULT _ReparentPseudoConsole(_In_ const PseudoConsole* const pPty, _In_ const HWND newParent); -void _ClosePseudoConsoleMembers(_In_ PseudoConsole* pPty, _In_ DWORD dwMilliseconds); +void _ClosePseudoConsoleMembers(_In_ PseudoConsole* pPty); HRESULT WINAPI ConptyCreatePseudoConsoleAsUser(_In_ HANDLE hToken, _In_ COORD size,