Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Align the OSS ConPTY API with Windows 11 24H2 #17704

Merged
merged 2 commits into from
Aug 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cascadia/TerminalConnection/ConptyConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,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
Expand Down
1 change: 0 additions & 1 deletion src/inc/conpty-static.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
2 changes: 1 addition & 1 deletion src/winconpty/dll/winconpty.def
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ EXPORTS
ConptyCreatePseudoConsoleAsUser
ConptyResizePseudoConsole
ConptyClosePseudoConsole
ConptyClosePseudoConsoleTimeout
ConptyClearPseudoConsole
ConptyShowHidePseudoConsole
ConptyReparentPseudoConsole
Expand All @@ -18,3 +17,4 @@ EXPORTS
ResizePseudoConsole = ConptyResizePseudoConsole
ClosePseudoConsole = ConptyClosePseudoConsole
ClearPseudoConsole = ConptyClearPseudoConsole
ReleasePseudoConsole = ConptyReleasePseudoConsole
17 changes: 9 additions & 8 deletions src/winconpty/ft_pty/ConPtyTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -212,7 +212,7 @@ void ConPtyTests::GoodCreate()
&pcon));

auto closePty = wil::scope_exit([&] {
_ClosePseudoConsoleMembers(&pcon, INFINITE);
_ClosePseudoConsoleMembers(&pcon);
});
}

Expand Down Expand Up @@ -241,7 +241,7 @@ void ConPtyTests::GoodCreateMultiple()
0,
&pcon1));
auto closePty1 = wil::scope_exit([&] {
_ClosePseudoConsoleMembers(&pcon1, INFINITE);
_ClosePseudoConsoleMembers(&pcon1);
});

VERIFY_SUCCEEDED(
Expand All @@ -251,7 +251,7 @@ void ConPtyTests::GoodCreateMultiple()
0,
&pcon2));
auto closePty2 = wil::scope_exit([&] {
_ClosePseudoConsoleMembers(&pcon2, INFINITE);
_ClosePseudoConsoleMembers(&pcon2);
});
}

Expand All @@ -278,7 +278,7 @@ void ConPtyTests::SurvivesOnBreakOutput()
0,
&pty));
auto closePty1 = wil::scope_exit([&] {
_ClosePseudoConsoleMembers(&pty, INFINITE);
_ClosePseudoConsoleMembers(&pty);
});

DWORD dwExit;
Expand Down Expand Up @@ -337,7 +337,7 @@ void ConPtyTests::DiesOnClose()
0,
&pty));
auto closePty1 = wil::scope_exit([&] {
_ClosePseudoConsoleMembers(&pty, INFINITE);
_ClosePseudoConsoleMembers(&pty);
});

DWORD dwExit;
Expand All @@ -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);
}
Expand Down
35 changes: 5 additions & 30 deletions src/winconpty/winconpty.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
// - <none>
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;
}
Expand All @@ -412,11 +399,11 @@ void _ClosePseudoConsoleMembers(_In_ PseudoConsole* pPty, _In_ DWORD dwMilliseco
// - wait: If true, waits for conhost/OpenConsole to exit first.
// Return Value:
// - <none>
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);
}
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/winconpty/winconpty.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading