terminal/src/host/PtySignalInputThread.cpp

241 lines
8 KiB
C++
Raw Normal View History

// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
#include "precomp.h"
#include "PtySignalInputThread.hpp"
#include "output.h"
#include "handle.h"
#include "../interactivity/inc/ServiceLocator.hpp"
#include "../terminal/adapter/DispatchCommon.hpp"
using namespace Microsoft::Console;
using namespace Microsoft::Console::Interactivity;
using namespace Microsoft::Console::VirtualTerminal;
// Constructor Description:
// - Creates the PTY Signal Input Thread.
// Arguments:
// - hPipe - a handle to the file representing the read end of the VT pipe.
Persist inbox conhost; delegate control activities to it via a pipe (#10415) Persist inbox conhost; delegate control activities to it via a pipe ## PR Checklist * [x] Closes #10194 - WSL Debug Tap doesn't work * [x] Closes #10134 - WSL Parameter is Incorrect * [x] Closes #10413 - Ctrl+C not passed to client * [x] Closes #10414 - Leftover processes on abrupt termination * [x] Might help #10251 - Win+X Powershell sometimes fails to attach * [x] I work here * [x] Manually tested with assorted launch scenarios ## Detailed Description of the Pull Request / Additional comments It turns out that there's a bit of ownership that goes on with the original inbox `conhost.exe` and the operating system/driver. The PID of that original `conhost.exe` is stowed when the initial connection is established and it is verified for several activities. This means that the plan of letting it go completely away and having the `OpenConsole.exe` take over all of its activities must be slightly revised. I have tested the following two alternatives to keeping `conhost.exe` around and they do not work: 1. Replacing the original owner `conhost.exe` with `OpenConsole.exe` - A.) The driver does not allow this. Once the owner is registered, it cannot be replaced. B.) There's no way of updating this information inside the client process space and it is kept there too in the `kernelbase`/`conclnt` data from its initial connection. 2. Attempting to pick up the first packet (to determine headed/headless and other initial connection information that we use to determine whether handoff is appropriate or not) prior to registering any owner at all. - The driver doesn't allow this either. The owner must be registered prior to a packet coming through. Put this mental model in your head: CMD --> Conhost (inbox) --> OpenConsole (WT Package) --> Terminal (WT Package) So since the `conhost.exe` needs to stick around, here's what I'm doing in this PR: - `conhost.exe` in the OS will receive back the `OpenConsole.exe` process handle on a successful handoff and is expected to remain alive until the `OpenConsole.exe` exits. It's now waiting on that before it terminates itself. - `conhost.exe` in the OS will establish a signal channel pipe and listen for control commands from `OpenConsole.exe` in a very similar fashion to how the `ConPTY` signal pipe operates between the Terminal and the PTY (provided by `OpenConsole.exe` in this particular example.) When `OpenConsole.exe` needs to do something that would be verified by the OS and rejected... it will instead signal the original `conhost.exe` to do that thing and it will go through. - `conhost.exe` will give its own handle through to `OpenConsole.exe` so it can monitor its lifetime and cleanup. If the owner is gone, the session should end. - Assorted handle cleanup that was leading to improper exits. I was confused between `.reset()` and `.release()` for some of the `wil::unique_any<T>` handling and it lead to leaked handles. The leaked handles meant that threads weren't aware of the other sides collapsing and wouldn't cleanup/terminate appropriately. How does this fix things? - For the WSL cases... WSL was specifically looking up the owner PID of the console session from the driver. That was the `conhost.exe` PID. If it exits, that PID isn't valid and is recycled. Thus the parameter is incorrect or other inappropriate WSL setup behaviors. - Ctrl+C not passed... this is a signal the operating system rejects from a PID that is not the owner. This is now relayed through the original owner and it works. - Leftover processes... I believe I explained this was both not-enough-monitoring of each others' process lifetimes coupled with mishandling of release/resetting handles and leaking them. - Powershell sometimes fails to attach... my theory on this one is that it's a race that became upset when the `conhost.exe` disappeared while something about Powershell/.NET was still starting, much like the WSL one. I believe now that it is sticking around, it will be fine. Also, this WILL require an OS update to complete improvement of functionality and I have revised the interface ID. This is considered an acceptable breaking change with no mitigation because we said this feature was an alpha preview. ## Validation Steps Performed - Launched WSL with defapp set, it works - Launched WSL with defapp set and the debug tap on, it works and opens in two tabs - Launched CMD, ran ping, did Ctrl+C, it now receives it - Launched Win+X powershell a ton of times. It seems fine now - Launched cmd, powershell, wsl, etc. Killed assorted processes in the chain (client/conhost/openconsole/windowsterminal) and observed in Process Explorer (with a long delta timer so I could see it) that they all successfully tear down now without leftovers.
2021-06-16 21:23:37 +02:00
PtySignalInputThread::PtySignalInputThread(wil::unique_hfile hPipe) :
_hFile{ std::move(hPipe) },
_hThread{},
_pConApi{ std::make_unique<ConhostInternalGetSet>(ServiceLocator::LocateGlobals().getConsoleInformation()) },
_dwThreadId{ 0 },
_consoleConnected{ false }
{
THROW_HR_IF(E_HANDLE, _hFile.get() == INVALID_HANDLE_VALUE);
THROW_HR_IF_NULL(E_INVALIDARG, _pConApi.get());
}
PtySignalInputThread::~PtySignalInputThread()
{
// Manually terminate our thread during unittesting. Otherwise, the test
// will finish, but TAEF will not manually kill the test.
#ifdef UNIT_TESTING
TerminateThread(_hThread.get(), 0);
#endif
}
// Function Description:
// - Static function used for initializing an instance's ThreadProc.
// Arguments:
// - lpParameter - A pointer to the PtySignalInputTHread instance that should be called.
// Return Value:
// - The return value of the underlying instance's _InputThread
DWORD WINAPI PtySignalInputThread::StaticThreadProc(_In_ LPVOID lpParameter)
{
PtySignalInputThread* const pInstance = reinterpret_cast<PtySignalInputThread*>(lpParameter);
return pInstance->_InputThread();
}
// Method Description:
// - Tell us that there's a client attached to the console, so we can actually
// 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:
// - The ThreadProc for the PTY Signal Input Thread.
// Return Value:
// - S_OK if the thread runs to completion.
Persist inbox conhost; delegate control activities to it via a pipe (#10415) Persist inbox conhost; delegate control activities to it via a pipe ## PR Checklist * [x] Closes #10194 - WSL Debug Tap doesn't work * [x] Closes #10134 - WSL Parameter is Incorrect * [x] Closes #10413 - Ctrl+C not passed to client * [x] Closes #10414 - Leftover processes on abrupt termination * [x] Might help #10251 - Win+X Powershell sometimes fails to attach * [x] I work here * [x] Manually tested with assorted launch scenarios ## Detailed Description of the Pull Request / Additional comments It turns out that there's a bit of ownership that goes on with the original inbox `conhost.exe` and the operating system/driver. The PID of that original `conhost.exe` is stowed when the initial connection is established and it is verified for several activities. This means that the plan of letting it go completely away and having the `OpenConsole.exe` take over all of its activities must be slightly revised. I have tested the following two alternatives to keeping `conhost.exe` around and they do not work: 1. Replacing the original owner `conhost.exe` with `OpenConsole.exe` - A.) The driver does not allow this. Once the owner is registered, it cannot be replaced. B.) There's no way of updating this information inside the client process space and it is kept there too in the `kernelbase`/`conclnt` data from its initial connection. 2. Attempting to pick up the first packet (to determine headed/headless and other initial connection information that we use to determine whether handoff is appropriate or not) prior to registering any owner at all. - The driver doesn't allow this either. The owner must be registered prior to a packet coming through. Put this mental model in your head: CMD --> Conhost (inbox) --> OpenConsole (WT Package) --> Terminal (WT Package) So since the `conhost.exe` needs to stick around, here's what I'm doing in this PR: - `conhost.exe` in the OS will receive back the `OpenConsole.exe` process handle on a successful handoff and is expected to remain alive until the `OpenConsole.exe` exits. It's now waiting on that before it terminates itself. - `conhost.exe` in the OS will establish a signal channel pipe and listen for control commands from `OpenConsole.exe` in a very similar fashion to how the `ConPTY` signal pipe operates between the Terminal and the PTY (provided by `OpenConsole.exe` in this particular example.) When `OpenConsole.exe` needs to do something that would be verified by the OS and rejected... it will instead signal the original `conhost.exe` to do that thing and it will go through. - `conhost.exe` will give its own handle through to `OpenConsole.exe` so it can monitor its lifetime and cleanup. If the owner is gone, the session should end. - Assorted handle cleanup that was leading to improper exits. I was confused between `.reset()` and `.release()` for some of the `wil::unique_any<T>` handling and it lead to leaked handles. The leaked handles meant that threads weren't aware of the other sides collapsing and wouldn't cleanup/terminate appropriately. How does this fix things? - For the WSL cases... WSL was specifically looking up the owner PID of the console session from the driver. That was the `conhost.exe` PID. If it exits, that PID isn't valid and is recycled. Thus the parameter is incorrect or other inappropriate WSL setup behaviors. - Ctrl+C not passed... this is a signal the operating system rejects from a PID that is not the owner. This is now relayed through the original owner and it works. - Leftover processes... I believe I explained this was both not-enough-monitoring of each others' process lifetimes coupled with mishandling of release/resetting handles and leaking them. - Powershell sometimes fails to attach... my theory on this one is that it's a race that became upset when the `conhost.exe` disappeared while something about Powershell/.NET was still starting, much like the WSL one. I believe now that it is sticking around, it will be fine. Also, this WILL require an OS update to complete improvement of functionality and I have revised the interface ID. This is considered an acceptable breaking change with no mitigation because we said this feature was an alpha preview. ## Validation Steps Performed - Launched WSL with defapp set, it works - Launched WSL with defapp set and the debug tap on, it works and opens in two tabs - Launched CMD, ran ping, did Ctrl+C, it now receives it - Launched Win+X powershell a ton of times. It seems fine now - Launched cmd, powershell, wsl, etc. Killed assorted processes in the chain (client/conhost/openconsole/windowsterminal) and observed in Process Explorer (with a long delta timer so I could see it) that they all successfully tear down now without leftovers.
2021-06-16 21:23:37 +02:00
// - Otherwise it may cause an application termination and never return.
[[nodiscard]] HRESULT PtySignalInputThread::_InputThread()
{
PtySignal signalId;
while (_GetData(&signalId, sizeof(signalId)))
{
switch (signalId)
{
case PtySignal::ClearBuffer:
{
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)
{
_DoClearBuffer();
}
break;
}
case PtySignal::ResizeWindow:
{
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)
{
_earlyResize = resizeMsg;
}
else
{
_DoResizeWindow(resizeMsg);
}
break;
}
default:
{
THROW_HR(E_UNEXPECTED);
}
}
}
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);
}
}
void PtySignalInputThread::_DoClearBuffer()
{
_pConApi->PrivateClearBuffer();
}
// Method Description:
// - Retrieves bytes from the file stream and exits or throws errors should the pipe state
// be compromised.
// Arguments:
// - pBuffer - Buffer to fill with data.
// - cbBuffer - Count of bytes in the given buffer.
// Return Value:
// - True if data was retrieved successfully. False otherwise.
bool PtySignalInputThread::_GetData(_Out_writes_bytes_(cbBuffer) void* const pBuffer,
const DWORD cbBuffer)
{
DWORD dwRead = 0;
// If we failed to read because the terminal broke our pipe (usually due
// to dying itself), close gracefully with ERROR_BROKEN_PIPE.
// Otherwise throw an exception. ERROR_BROKEN_PIPE is the only case that
// we want to gracefully close in.
if (FALSE == ReadFile(_hFile.get(), pBuffer, cbBuffer, &dwRead, nullptr))
{
DWORD lastError = GetLastError();
if (lastError == ERROR_BROKEN_PIPE)
{
_Shutdown();
return false;
}
else
{
THROW_WIN32(lastError);
}
}
else if (dwRead != cbBuffer)
{
_Shutdown();
return false;
}
return true;
}
// Method Description:
// - Starts the PTY Signal input thread.
[[nodiscard]] HRESULT PtySignalInputThread::Start() noexcept
{
RETURN_LAST_ERROR_IF(!_hFile);
HANDLE hThread = nullptr;
// 0 is the right value, https://blogs.msdn.microsoft.com/oldnewthing/20040223-00/?p=40503
DWORD dwThreadId = 0;
hThread = CreateThread(nullptr,
0,
PtySignalInputThread::StaticThreadProc,
this,
0,
&dwThreadId);
RETURN_LAST_ERROR_IF_NULL(hThread);
_hThread.reset(hThread);
_dwThreadId = dwThreadId;
LOG_IF_FAILED(SetThreadDescription(hThread, L"ConPTY Signal Handler Thread"));
return S_OK;
}
// Method Description:
// - Perform a shutdown of the console. This happens when the signal pipe is
// broken, which means either the parent terminal process has died, or they
// called ClosePseudoConsole.
// CloseConsoleProcessState is not enough by itself - it will disconnect
// clients as if the X was pressed, but then we need to actually still die,
// so then call RundownAndExit.
// Arguments:
// - <none>
// Return Value:
// - <none>
void PtySignalInputThread::_Shutdown()
{
// Trigger process shutdown.
CloseConsoleProcessState();
// If we haven't terminated by now, that's because there's a client that's still attached.
// Force the handling of the control events by the attached clients.
// As of MSFT:19419231, CloseConsoleProcessState will make sure this
// happens if this method is called outside of lock, but if we're
// currently locked, we want to make sure ctrl events are handled
// _before_ we RundownAndExit.
ProcessCtrlEvents();
// Make sure we terminate.
ServiceLocator::RundownAndExit(ERROR_BROKEN_PIPE);
}