diff --git a/src/host/VtIo.cpp b/src/host/VtIo.cpp index 1b8a8591d2..be1d8bd330 100644 --- a/src/host/VtIo.cpp +++ b/src/host/VtIo.cpp @@ -79,7 +79,11 @@ using namespace Microsoft::Console::Interactivity; const HANDLE OutHandle, _In_opt_ const HANDLE SignalHandle) { - FAIL_FAST_IF_MSG(_initialized, "Someone attempted to double-_Initialize VtIo"); + if (_state != State::Uninitialized) + { + assert(false); // Don't call initialize twice. + return E_UNEXPECTED; + } _hInput.reset(InHandle); _hOutput.reset(OutHandle); @@ -95,47 +99,33 @@ using namespace Microsoft::Console::Interactivity; } } + // - Create and start the signal thread. The signal thread can be created + // independent of the i/o threads, and doesn't require a client first + // attaching to the console. We need to create it first and foremost, + // because it's possible that a terminal application could + // CreatePseudoConsole, then ClosePseudoConsole without ever attaching a + // client. Should that happen, we still need to exit. + if (IsValidHandle(_hSignal.get())) + { + try + { + _pPtySignalInputThread = std::make_unique(std::move(_hSignal)); + + // Start it if it was successfully created. + RETURN_IF_FAILED(_pPtySignalInputThread->Start()); + } + CATCH_RETURN(); + } + // The only way we're initialized is if the args said we're in conpty mode. // If the args say so, then at least one of in, out, or signal was specified - _initialized = true; - return S_OK; -} - -// Method Description: -// - Create the VtEngine and the VtInputThread for this console. -// MUST BE DONE AFTER CONSOLE IS INITIALIZED, to make sure we've gotten the -// buffer size from the attached client application. -// Arguments: -// - -// Return Value: -// S_OK if we initialized successfully, -// S_FALSE if VtIo hasn't been initialized (or we're not in conpty mode) -// otherwise an appropriate HRESULT indicating failure. -[[nodiscard]] HRESULT VtIo::CreateIoHandlers() noexcept -{ - if (!_initialized) - { - return S_FALSE; - } - - // SetWindowVisibility uses the console lock to protect access to _pVtRenderEngine. - assert(ServiceLocator::LocateGlobals().getConsoleInformation().IsConsoleLocked()); - - try - { - if (IsValidHandle(_hInput.get())) - { - _pVtInputThread = std::make_unique(std::move(_hInput), _lookingForCursorPosition); - } - } - CATCH_RETURN(); - + _state = State::Initialized; return S_OK; } bool VtIo::IsUsingVt() const { - return _initialized; + return _state != State::Uninitialized; } // Routine Description: @@ -151,50 +141,64 @@ bool VtIo::IsUsingVt() const [[nodiscard]] HRESULT VtIo::StartIfNeeded() { // If we haven't been set up, do nothing (because there's nothing to start) - if (!_initialized) + if (_state != State::Initialized) { return S_FALSE; } + _state = State::Starting; + + // SetWindowVisibility uses the console lock to protect access to _pVtRenderEngine. + assert(ServiceLocator::LocateGlobals().getConsoleInformation().IsConsoleLocked()); + + try + { + if (IsValidHandle(_hInput.get())) + { + _pVtInputThread = std::make_unique(std::move(_hInput), _lookingForCursorPosition); + } + } + CATCH_RETURN(); + if (_pVtInputThread) { LOG_IF_FAILED(_pVtInputThread->Start()); - } - { - Writer writer{ this }; - - // 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. - // If we get a response, the InteractDispatch will call SetCursorPosition, - // which will call to our VtIo::SetCursorPosition method. - // - // By sending the request before sending the DA1 one, we can simply - // wait for the DA1 response below and effectively wait for both. - if (_lookingForCursorPosition) { - writer.WriteUTF8("\x1b[6n"); // Cursor Position Report (DSR CPR) + Writer writer{ this }; + + // 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. + // If we get a response, the InteractDispatch will call SetCursorPosition, + // which will call to our VtIo::SetCursorPosition method. + // + // By sending the request before sending the DA1 one, we can simply + // wait for the DA1 response below and effectively wait for both. + if (_lookingForCursorPosition) + { + writer.WriteUTF8("\x1b[6n"); // Cursor Position Report (DSR CPR) + } + + // GH#4999 - Send a sequence to the connected terminal to request + // win32-input-mode from them. This will enable the connected terminal to + // send us full INPUT_RECORDs as input. If the terminal doesn't understand + // this sequence, it'll just ignore it. + writer.WriteUTF8( + "\x1b[c" // DA1 Report (Primary Device Attributes) + "\x1b[?1004h" // Focus Event Mode + "\x1b[?9001h" // Win32 Input Mode + ); + + writer.Submit(); } - // GH#4999 - Send a sequence to the connected terminal to request - // win32-input-mode from them. This will enable the connected terminal to - // send us full INPUT_RECORDs as input. If the terminal doesn't understand - // this sequence, it'll just ignore it. - writer.WriteUTF8( - "\x1b[c" // DA1 Report (Primary Device Attributes) - "\x1b[?1004h" // Focus Event Mode - "\x1b[?9001h" // Win32 Input Mode - ); - - writer.Submit(); - } - - { - // Allow the input thread to momentarily gain the console lock. - auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - const auto suspension = gci.SuspendLock(); - _deviceAttributes = _pVtInputThread->WaitUntilDA1(3000); + { + // Allow the input thread to momentarily gain the console lock. + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + const auto suspension = gci.SuspendLock(); + _deviceAttributes = _pVtInputThread->WaitUntilDA1(3000); + } } if (_pPtySignalInputThread) @@ -208,6 +212,17 @@ bool VtIo::IsUsingVt() const _pPtySignalInputThread->ConnectConsole(); } + if (_state != State::Starting) + { + // Here's where we _could_ call CloseConsoleProcessState(), but this function + // only gets called once when the first client connects and CONSOLE_INITIALIZED + // is not set yet. The process list may already contain that first client, + // but since it hasn't finished connecting yet, it won't react to a CTRL_CLOSE_EVENT. + // Instead, we return an error here which will abort the connection setup. + return E_FAIL; + } + + _state = State::Running; return S_OK; } @@ -244,47 +259,21 @@ void VtIo::CreatePseudoWindow() } } -// Method Description: -// - Create and start the signal thread. The signal thread can be created -// independent of the i/o threads, and doesn't require a client first -// attaching to the console. We need to create it first and foremost, -// because it's possible that a terminal application could -// CreatePseudoConsole, then ClosePseudoConsole without ever attaching a -// client. Should that happen, we still need to exit. -// Arguments: -// - -// Return Value: -// - S_FALSE if we're not in VtIo mode, -// S_OK if we succeeded, -// otherwise an appropriate HRESULT indicating failure. -[[nodiscard]] HRESULT VtIo::CreateAndStartSignalThread() noexcept -{ - if (!_initialized) - { - return S_FALSE; - } - - // If we were passed a signal handle, try to open it and make a signal reading thread. - if (IsValidHandle(_hSignal.get())) - { - try - { - _pPtySignalInputThread = std::make_unique(std::move(_hSignal)); - - // Start it if it was successfully created. - RETURN_IF_FAILED(_pPtySignalInputThread->Start()); - } - CATCH_RETURN(); - } - - return S_OK; -} - void VtIo::SendCloseEvent() { LockConsole(); const auto unlock = wil::scope_exit([] { UnlockConsole(); }); + // If we're still in the process of starting up, and we're asked to shut down + // (broken pipe), `VtIo::StartIfNeeded()` will handle the cleanup for us. + // This can happen during the call to `WaitUntilDA1`, because we relinquish + // ownership of the console lock. + if (_state == State::Starting) + { + _state = State::StartupFailed; + return; + } + // This function is called when the ConPTY signal pipe is closed (PtySignalInputThread) and when the input // pipe is closed (VtIo). Usually these two happen at about the same time. This if condition is a bit of // a premature optimization and prevents us from sending out a CTRL_CLOSE_EVENT right after another. diff --git a/src/host/VtIo.hpp b/src/host/VtIo.hpp index 4e9d250c4c..6805fbc730 100644 --- a/src/host/VtIo.hpp +++ b/src/host/VtIo.hpp @@ -57,8 +57,6 @@ namespace Microsoft::Console::VirtualTerminal static wchar_t SanitizeUCS2(wchar_t ch); [[nodiscard]] HRESULT Initialize(const ConsoleArguments* const pArgs); - [[nodiscard]] HRESULT CreateAndStartSignalThread() noexcept; - [[nodiscard]] HRESULT CreateIoHandlers() noexcept; bool IsUsingVt() const; [[nodiscard]] HRESULT StartIfNeeded(); @@ -69,6 +67,15 @@ namespace Microsoft::Console::VirtualTerminal void CreatePseudoWindow(); private: + enum class State : uint8_t + { + Uninitialized, + Initialized, + Starting, + StartupFailed, + Running, + }; + [[nodiscard]] HRESULT _Initialize(const HANDLE InHandle, const HANDLE OutHandle, _In_opt_ const HANDLE SignalHandle); void _uncork(); @@ -77,7 +84,7 @@ namespace Microsoft::Console::VirtualTerminal // After CreateIoHandlers is called, these will be invalid. wil::unique_hfile _hInput; wil::unique_hfile _hOutput; - // After CreateAndStartSignalThread is called, this will be invalid. + // After Initialize is called, this will be invalid. wil::unique_hfile _hSignal; std::unique_ptr _pVtInputThread; @@ -96,7 +103,7 @@ namespace Microsoft::Console::VirtualTerminal bool _writerRestoreCursor = false; bool _writerTainted = false; - bool _initialized = false; + State _state = State::Uninitialized; bool _lookingForCursorPosition = false; bool _closeEventSent = false; int _corked = 0; diff --git a/src/host/srvinit.cpp b/src/host/srvinit.cpp index 5b10705a63..97bcd8f276 100644 --- a/src/host/srvinit.cpp +++ b/src/host/srvinit.cpp @@ -381,7 +381,6 @@ HRESULT ConsoleCreateIoThread(_In_ HANDLE Server, // can start, so they're started below, in ConsoleAllocateConsole auto& gci = g.getConsoleInformation(); RETURN_IF_FAILED(gci.GetVtIo()->Initialize(args)); - RETURN_IF_FAILED(gci.GetVtIo()->CreateAndStartSignalThread()); return S_OK; } @@ -945,27 +944,11 @@ PWSTR TranslateConsoleTitle(_In_ PCWSTR pwszConsoleTitle, const BOOL fUnexpand, // We'll need the size of the screen buffer in the vt i/o initialization if (SUCCEEDED_NTSTATUS(Status)) { - auto hr = gci.GetVtIo()->CreateIoHandlers(); - if (hr == S_FALSE) - { - // We're not in VT I/O mode, this is fine. - } - else if (SUCCEEDED(hr)) - { - // Actually start the VT I/O threads - hr = gci.GetVtIo()->StartIfNeeded(); - // Don't convert S_FALSE to an NTSTATUS - the equivalent NTSTATUS - // is treated as an error - if (hr != S_FALSE) - { - Status = NTSTATUS_FROM_HRESULT(hr); - } - else - { - Status = ERROR_SUCCESS; - } - } - else + // Actually start the VT I/O threads + auto hr = gci.GetVtIo()->StartIfNeeded(); + // Don't convert S_FALSE to an NTSTATUS - the equivalent NTSTATUS + // is treated as an error + if (FAILED(hr)) { Status = NTSTATUS_FROM_HRESULT(hr); }