microsoft/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2025-12-10 18:43:54 -06:00
Code Issues Packages Projects Releases 138 Wiki Activity

Implement a timeout for PSEUDOCONSOLE_INHERIT_CURSOR (#17574)

Browse Source 
This implements a 3s timeout for cursor inheritance which prevents
ConPTY from being deadlocked at startup, if the terminal misbehaves.
It serves another purpose, however, in that it prepares the code for
the introduction of overlapped IO in #17510.

Closes #11213
This commit is contained in:
Leonard Hecker 2024-07-17 03:02:59 +02:00 committed by GitHub
parent 2769bb591b
commit 1ac221a7a5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 89 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

150
src/host/VtInputThread.cpp
View File

@ -2,17 +2,15 @@
// Licensed under the MIT license.
#include "precomp.h"
#include "VtInputThread.hpp"
#include "../interactivity/inc/ServiceLocator.hpp"
#include "input.h"
#include "../terminal/parser/InputStateMachineEngine.hpp"
#include "../terminal/adapter/InteractDispatch.hpp"
#include "../types/inc/convert.hpp"
#include "server.h"
#include "output.h"
#include "handle.h"
#include "output.h"
#include "server.h"
#include "../interactivity/inc/ServiceLocator.hpp"
#include "../terminal/adapter/InteractDispatch.hpp"
#include "../terminal/parser/InputStateMachineEngine.hpp"
#include "../types/inc/utils.hpp"
using namespace Microsoft::Console;
using namespace Microsoft::Console::Interactivity;
@ -23,30 +21,18 @@ using namespace Microsoft::Console::VirtualTerminal;
// - hPipe - a handle to the file representing the read end of the VT pipe.
// - inheritCursor - a bool indicating if the state machine should expect a
// cursor positioning sequence. See MSFT:15681311.
VtInputThread::VtInputThread(_In_ wil::unique_hfile hPipe,
const bool inheritCursor) :
_hFile{ std::move(hPipe) },
_hThread{},
_u8State{},
_dwThreadId{ 0 },
_pfnSetLookingForDSR{}
VtInputThread::VtInputThread(_In_ wil::unique_hfile hPipe, const bool inheritCursor) :
_hFile{ std::move(hPipe) }
{
THROW_HR_IF(E_HANDLE, _hFile.get() == INVALID_HANDLE_VALUE);
auto dispatch = std::make_unique<InteractDispatch>();
auto engine = std::make_unique<InputStateMachineEngine>(std::move(dispatch), inheritCursor);
auto engineRef = engine.get();
// we need this callback to be able to flush an unknown input sequence to the app
engine->SetFlushToInputQueueCallback([this] { return _pInputStateMachine->FlushToTerminal(); });
_pInputStateMachine = std::make_unique<StateMachine>(std::move(engine));
// we need this callback to be able to flush an unknown input sequence to the app
auto flushCallback = [capture0 = _pInputStateMachine.get()] { return capture0->FlushToTerminal(); };
engineRef->SetFlushToInputQueueCallback(flushCallback);
// we need this callback to capture the reply if someone requests a status from the terminal
_pfnSetLookingForDSR = [engineRef](auto&& PH1) { engineRef->SetLookingForDSR(std::forward<decltype(PH1)>(PH1)); };
}
// Function Description:
@ -57,79 +43,67 @@ VtInputThread::VtInputThread(_In_ wil::unique_hfile hPipe,
// - The return value of the underlying instance's _InputThread
DWORD WINAPI VtInputThread::StaticVtInputThreadProc(_In_ LPVOID lpParameter)
{
const auto pInstance = reinterpret_cast<VtInputThread*>(lpParameter);
const auto pInstance = static_cast<VtInputThread*>(lpParameter);
pInstance->_InputThread();
return S_OK;
}
// Method Description:
// - Do a single ReadFile from our pipe, and try and handle it. If handling
// failed, throw or log, depending on what the caller wants.
// Return Value:
// - true if you should continue reading
bool VtInputThread::DoReadInput()
{
char buffer[4096];
DWORD dwRead = 0;
const auto ok = ReadFile(_hFile.get(), buffer, ARRAYSIZE(buffer), &dwRead, nullptr);
// The ReadFile() documentations calls out that:
// > If the lpNumberOfBytesRead parameter is zero when ReadFile returns TRUE on a pipe, the other
// > end of the pipe called the WriteFile function with nNumberOfBytesToWrite set to zero.
// But I was unable to replicate any such behavior. I'm not sure it's true anymore.
//
// However, what the documentations fails to mention is that winsock2 (WSA) handles of the \Device\Afd type are
// transparently compatible with ReadFile() and the WSARecv() documentations contains this important information:
// > For byte streams, zero bytes having been read [..] indicates graceful closure and that no more bytes will ever be read.
// In other words, for pipe HANDLE of unknown type you should consider `lpNumberOfBytesRead == 0` as an exit indicator.
//
// Here, `dwRead == 0` fixes a deadlock when exiting conhost while being in use by WSL whose hypervisor pipes are WSA.
if (!ok || dwRead == 0)
{
return false;
}
// If we hit a parsing error, eat it. It's bad utf-8, we can't do anything with it.
if (FAILED_LOG(til::u8u16({ buffer, gsl::narrow_cast<size_t>(dwRead) }, _wstr, _u8State)))
{
return true;
}
try
{
// Make sure to call the GLOBAL Lock/Unlock, not the gci's lock/unlock.
// Only the global unlock attempts to dispatch ctrl events. If you use the
// gci's unlock, when you press C-c, it won't be dispatched until the
// next console API call. For something like `powershell sleep 60`,
// that won't happen for 60s
LockConsole();
const auto unlock = wil::scope_exit([&] { UnlockConsole(); });
_pInputStateMachine->ProcessString(_wstr);
}
CATCH_LOG();
return true;
}
void VtInputThread::SetLookingForDSR(const bool looking) noexcept
{
if (_pfnSetLookingForDSR)
{
_pfnSetLookingForDSR(looking);
}
}
// Method Description:
// - The ThreadProc for the VT Input Thread. Reads input from the pipe, and
// passes it to _HandleRunInput to be processed by the
// InputStateMachineEngine.
void VtInputThread::_InputThread()
{
while (DoReadInput())
const auto cleanup = wil::scope_exit([this]() {
ServiceLocator::LocateGlobals().getConsoleInformation().GetVtIo()->CloseInput();
});
char buffer[4096];
DWORD dwRead = 0;
til::u8state u8State;
std::wstring wstr;
for (;;)
{
const auto ok = ReadFile(_hFile.get(), buffer, ARRAYSIZE(buffer), &dwRead, nullptr);
// The ReadFile() documentations calls out that:
// > If the lpNumberOfBytesRead parameter is zero when ReadFile returns TRUE on a pipe, the other
// > end of the pipe called the WriteFile function with nNumberOfBytesToWrite set to zero.
// But I was unable to replicate any such behavior. I'm not sure it's true anymore.
//
// However, what the documentations fails to mention is that winsock2 (WSA) handles of the \Device\Afd type are
// transparently compatible with ReadFile() and the WSARecv() documentations contains this important information:
// > For byte streams, zero bytes having been read [..] indicates graceful closure and that no more bytes will ever be read.
// In other words, for pipe HANDLE of unknown type you should consider `lpNumberOfBytesRead == 0` as an exit indicator.
//
// Here, `dwRead == 0` fixes a deadlock when exiting conhost while being in use by WSL whose hypervisor pipes are WSA.
if (!ok || dwRead == 0)
{
break;
}
// If we hit a parsing error, eat it. It's bad utf-8, we can't do anything with it.
if (FAILED_LOG(til::u8u16({ buffer, gsl::narrow_cast<size_t>(dwRead) }, wstr, u8State)))
{
continue;
}
try
{
// Make sure to call the GLOBAL Lock/Unlock, not the gci's lock/unlock.
// Only the global unlock attempts to dispatch ctrl events. If you use the
// gci's unlock, when you press C-c, it won't be dispatched until the
// next console API call. For something like `powershell sleep 60`,
// that won't happen for 60s
LockConsole();
const auto unlock = wil::scope_exit([&] { UnlockConsole(); });
_pInputStateMachine->ProcessString(wstr);
}
CATCH_LOG();
}
ServiceLocator::LocateGlobals().getConsoleInformation().GetVtIo()->CloseInput();
}
// Method Description:
@ -156,3 +130,9 @@ void VtInputThread::_InputThread()
return S_OK;
}
void VtInputThread::WaitUntilDSR(DWORD timeout) const noexcept
{
const auto& engine = static_cast<InputStateMachineEngine&>(_pInputStateMachine->Engine());
engine.WaitUntilDSR(timeout);
}

11
src/host/VtInputThread.hpp
View File

@ -24,21 +24,16 @@ namespace Microsoft::Console
VtInputThread(_In_ wil::unique_hfile hPipe, const bool inheritCursor);
[[nodiscard]] HRESULT Start();
static DWORD WINAPI StaticVtInputThreadProc(_In_ LPVOID lpParameter);
bool DoReadInput();
void SetLookingForDSR(const bool looking) noexcept;
void WaitUntilDSR(DWORD timeout) const noexcept;
private:
static DWORD WINAPI StaticVtInputThreadProc(_In_ LPVOID lpParameter);
void _InputThread();
wil::unique_hfile _hFile;
wil::unique_handle _hThread;
DWORD _dwThreadId;
std::function<void(bool)> _pfnSetLookingForDSR;
DWORD _dwThreadId = 0;
std::unique_ptr<Microsoft::Console::VirtualTerminal::StateMachine> _pInputStateMachine;
til::u8state _u8State;
std::wstring _wstr;
};
}

11
src/host/VtIo.cpp
View File

@ -186,19 +186,14 @@ bool VtIo::IsUsingVt() const
// MSFT: 15813316
// If the terminal application wants us to inherit the cursor position,
// we're going to emit a VT sequence to ask for the cursor position, then
// read input until we get a response. Terminals who request this behavior
// but don't respond will hang.
// wait 3s until we get a response.
// If we get a response, the InteractDispatch will call SetCursorPosition,
// which will call to our VtIo::SetCursorPosition method.
// We need both handles for this initialization to work. If we don't have
// both, we'll skip it. They either aren't going to be reading output
// (so they can't get the DSR) or they can't write the response to us.
if (_lookingForCursorPosition && _pVtRenderEngine && _pVtInputThread)
{
_lookingForCursorPosition = false;
LOG_IF_FAILED(_pVtRenderEngine->RequestCursor());
while (_lookingForCursorPosition && _pVtInputThread->DoReadInput())
{
}
_pVtInputThread->WaitUntilDSR(3000);
}
// GH#4999 - Send a sequence to the connected terminal to request

23
src/terminal/parser/InputStateMachineEngine.cpp
View File

@ -6,8 +6,9 @@
#include "stateMachine.hpp"
#include "InputStateMachineEngine.hpp"
#include <til/atomic.h>
#include "../../inc/unicode.hpp"
#include "ascii.hpp"
#include "../../interactivity/inc/VtApiRedirection.hpp"
using namespace Microsoft::Console::VirtualTerminal;
@ -102,16 +103,21 @@ InputStateMachineEngine::InputStateMachineEngine(std::unique_ptr<IInteractDispat
THROW_HR_IF_NULL(E_INVALIDARG, _pDispatch.get());
}
void InputStateMachineEngine::WaitUntilDSR(DWORD timeout) const noexcept
{
// Technically we should decrement the timeout with each iteration,
// but I suspect infinite spurious wake-ups are a theoretical problem.
while (_lookingForDSR.load(std::memory_order::relaxed))
{
til::atomic_wait(_lookingForDSR, true, timeout);
}
}
bool InputStateMachineEngine::EncounteredWin32InputModeSequence() const noexcept
{
return _encounteredWin32InputModeSequence;
}
void InputStateMachineEngine::SetLookingForDSR(const bool looking) noexcept
{
_lookingForDSR = looking;
}
// Method Description:
// - Triggers the Execute action to indicate that the listener should
// immediately respond to a C0 control character.
@ -408,12 +414,13 @@ bool InputStateMachineEngine::ActionCsiDispatch(const VTID id, const VTParameter
// The F3 case is special - it shares a code with the DeviceStatusResponse.
// If we're looking for that response, then do that, and break out.
// Else, fall though to the _GetCursorKeysModifierState handler.
if (_lookingForDSR)
if (_lookingForDSR.load(std::memory_order::relaxed))
{
success = _pDispatch->MoveCursor(parameters.at(0), parameters.at(1));
// Right now we're only looking for on initial cursor
// position response. After that, only look for F3.
_lookingForDSR = false;
_lookingForDSR.store(false, std::memory_order::relaxed);
til::atomic_notify_all(_lookingForDSR);
break;
}
[[fallthrough]];

5
src/terminal/parser/InputStateMachineEngine.hpp
View File

@ -132,8 +132,9 @@ namespace Microsoft::Console::VirtualTerminal
InputStateMachineEngine(std::unique_ptr<IInteractDispatch> pDispatch,
const bool lookingForDSR);
void WaitUntilDSR(DWORD timeout) const noexcept;
bool EncounteredWin32InputModeSequence() const noexcept override;
void SetLookingForDSR(const bool looking) noexcept;
bool ActionExecute(const wchar_t wch) override;
bool ActionExecuteFromEscape(const wchar_t wch) override;
@ -165,7 +166,7 @@ namespace Microsoft::Console::VirtualTerminal
private:
const std::unique_ptr<IInteractDispatch> _pDispatch;
std::function<bool()> _pfnFlushToInputQueue;
bool _lookingForDSR;
std::atomic<bool> _lookingForDSR{ false };
bool _encounteredWin32InputModeSequence = false;
DWORD _mouseButtonState = 0;
std::chrono::milliseconds _doubleClickTime;