From 9054c819349fa8a052f52ed4d03c5656bec4ff9c Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Thu, 16 May 2024 00:03:04 +0200 Subject: [PATCH 1/2] Fix persistence of handoff'd tabs (#17268) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As it turns out, for handoff'd connections `Initialize` isn't called and this meant the `_sessionId` was always null. After this PR we still don't have a `_profileGuid` but that's probably not a critical issue, since that's an inherent flaw with handoff. It can only be solved in a robust manner if WT gets launched before the console app is launched, but it's unlikely for that to ever happen. ## Validation Steps Performed * Launch * Register that version of WT as the default * Close all tabs (Ctrl+Shift+W) * `persistedWindowLayouts` is empty ✅ * Launch cmd/pwsh via handoff * You get 1 window ✅ * Close the window (= press the X button) * Launch * You get 2 windows ✅ --- src/cascadia/TerminalApp/TerminalPage.cpp | 7 +++++-- src/cascadia/TerminalConnection/ConptyConnection.cpp | 4 +++- src/cascadia/TerminalConnection/ConptyConnection.h | 2 +- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 3a82a05aebf..ca86bb80a37 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -1899,7 +1899,10 @@ namespace winrt::TerminalApp::implementation void TerminalPage::PersistState() { - if (_startupState != StartupState::Initialized) + // This method may be called for a window even if it hasn't had a tab yet or lost all of them. + // We shouldn't persist such windows. + const auto tabCount = _tabs.Size(); + if (_startupState != StartupState::Initialized || tabCount == 0) { return; } @@ -1915,7 +1918,7 @@ namespace winrt::TerminalApp::implementation // if the focused tab was not the last tab, restore that auto idx = _GetFocusedTabIndex(); - if (idx && idx != _tabs.Size() - 1) + if (idx && idx != tabCount - 1) { ActionAndArgs action; action.Action(ShortcutAction::SwitchToTab); diff --git a/src/cascadia/TerminalConnection/ConptyConnection.cpp b/src/cascadia/TerminalConnection/ConptyConnection.cpp index dd85388263b..7d326ea65c1 100644 --- a/src/cascadia/TerminalConnection/ConptyConnection.cpp +++ b/src/cascadia/TerminalConnection/ConptyConnection.cpp @@ -180,12 +180,14 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation const HANDLE hRef, const HANDLE hServerProcess, const HANDLE hClientProcess, - TERMINAL_STARTUP_INFO startupInfo) : + const TERMINAL_STARTUP_INFO& startupInfo) : _rows{ 25 }, _cols{ 80 }, _inPipe{ hIn }, _outPipe{ hOut } { + _sessionId = Utils::CreateGuid(); + THROW_IF_FAILED(ConptyPackPseudoConsole(hServerProcess, hRef, hSig, &_hPC)); _piClient.hProcess = hClientProcess; diff --git a/src/cascadia/TerminalConnection/ConptyConnection.h b/src/cascadia/TerminalConnection/ConptyConnection.h index e1b47bee64e..246621c5eeb 100644 --- a/src/cascadia/TerminalConnection/ConptyConnection.h +++ b/src/cascadia/TerminalConnection/ConptyConnection.h @@ -19,7 +19,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation const HANDLE hRef, const HANDLE hServerProcess, const HANDLE hClientProcess, - TERMINAL_STARTUP_INFO startupInfo); + const TERMINAL_STARTUP_INFO& startupInfo); ConptyConnection() noexcept = default; void Initialize(const Windows::Foundation::Collections::ValueSet& settings); From 183a8956f699e1967d4cb4712184d0dbc395a0e5 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Thu, 16 May 2024 00:03:28 +0200 Subject: [PATCH 2/2] Fix lock warning during ReturnResponse (#17266) As reported here: https://github.com/microsoft/terminal/pull/16224#discussion_r1594849244 The underlying `WriteFile` call may block indefinitely and we shouldn't hold the terminal lock during that period. --- src/cascadia/TerminalCore/TerminalApi.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cascadia/TerminalCore/TerminalApi.cpp b/src/cascadia/TerminalCore/TerminalApi.cpp index 42f1c903c33..9b85b18d5ff 100644 --- a/src/cascadia/TerminalCore/TerminalApi.cpp +++ b/src/cascadia/TerminalCore/TerminalApi.cpp @@ -22,8 +22,9 @@ TRACELOGGING_DEFINE_PROVIDER(g_hCTerminalCoreProvider, void Terminal::ReturnResponse(const std::wstring_view response) { - if (_pfnWriteInput) + if (_pfnWriteInput && !response.empty()) { + const auto suspension = _readWriteLock.suspend(); _pfnWriteInput(response); } }