From 42b2ab43d201f604b579bc76d7b586f97a546364 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Wed, 6 May 2026 17:51:50 -0700 Subject: [PATCH 1/2] Fix crash when closing scratchpad pane --- src/cascadia/TerminalApp/Pane.cpp | 11 +++++++++++ src/cascadia/TerminalApp/Pane.h | 6 ++++++ src/cascadia/TerminalApp/TabManagement.cpp | 22 ++++++++++++++++++++++ 3 files changed, 39 insertions(+) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 85354101b68..c3536a77462 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -997,6 +997,17 @@ void Pane::_ContentLostFocusHandler(const winrt::Windows::Foundation::IInspectab // - void Pane::Close() { + // GH#20187: Make Close() idempotent. The actual removal of this pane + // from the parent's tree is deferred until the close animation in + // _CloseChildRoutine completes (~200ms). Without this guard, a second + // close request that reaches this same pane during the animation + // window (e.g. auto-repeat or rapid double-press of the closePane + // keybinding) would re-raise Closed and kick off a redundant close + // animation on the parent. + if (!_content) + { + return; + } _setPaneContent(nullptr); // Fire our Closed event to tell our parent that we should be removed. Closed.raise(nullptr, nullptr); diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index ecc81fad82f..b44925aea97 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -93,6 +93,12 @@ class Pane : public std::enable_shared_from_this winrt::Windows::UI::Xaml::Controls::Grid GetRootElement(); winrt::TerminalApp::IPaneContent GetContent() const noexcept { return _IsLeaf() ? _content : nullptr; } + // GH#20187: True if this is a leaf pane whose content has already been + // torn down by Pane::Close() but which has not yet been removed from + // its parent (the close animation in _CloseChildRoutine is still + // running). Such a "corpse" pane is not safe to operate on. + bool IsClosed() const noexcept { return _IsLeaf() && _content == nullptr; } + bool WasLastFocused() const noexcept; void UpdateVisuals(); void ClearActive(); diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp index 74b6c736939..35a0accbdd9 100644 --- a/src/cascadia/TerminalApp/TabManagement.cpp +++ b/src/cascadia/TerminalApp/TabManagement.cpp @@ -781,6 +781,16 @@ namespace winrt::TerminalApp::implementation // - pane: the pane to close. void TerminalPage::_HandleClosePaneRequested(std::shared_ptr pane) { + // GH#20187: Defensive guard. Callers should already filter out + // corpse panes (see _CloseFocusedPane), but in case a future + // caller passes a leaf pane whose content has already been torn + // down by a prior Pane::Close(), bail before we try to call + // GetTerminalArgsForPane on it. + if (pane->IsClosed()) + { + return; + } + // Build the list of actions to recreate the closed pane, // BuildStartupActions returns the "first" pane and the rest of // its actions are assuming that first pane has been created first. @@ -814,6 +824,18 @@ namespace winrt::TerminalApp::implementation if (const auto pane{ activeTab->GetActivePane() }) { + // GH#20187: When closePane is auto-repeated (or rapidly + // re-pressed), the second invocation can arrive while the + // parent's close animation is still running and + // Tab::_activePane still points at the already-closed + // leaf. Detect that "corpse" state here and bail before + // we try to call GetTerminalArgsForPane on a leaf whose + // content has been torn down. + if (pane->IsClosed()) + { + co_return; + } + const auto weak = get_weak(); // Check if we should warn before closing a single pane From 48bdb8c0cabcec0cc50200ce8d100f170cbb0ba8 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Wed, 6 May 2026 19:27:56 -0700 Subject: [PATCH 2/2] simplify --- src/cascadia/TerminalApp/Pane.cpp | 9 +++---- src/cascadia/TerminalApp/TabManagement.cpp | 29 +++++++++------------- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index c3536a77462..ebbe5865b33 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -999,11 +999,10 @@ void Pane::Close() { // GH#20187: Make Close() idempotent. The actual removal of this pane // from the parent's tree is deferred until the close animation in - // _CloseChildRoutine completes (~200ms). Without this guard, a second - // close request that reaches this same pane during the animation - // window (e.g. auto-repeat or rapid double-press of the closePane - // keybinding) would re-raise Closed and kick off a redundant close - // animation on the parent. + // _CloseChildRoutine completes (~200ms). Without this guard, a + // second close request that reaches this same pane during the + // animation window would re-raise Closed and kick off a redundant + // close animation on the parent. if (!_content) { return; diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp index 35a0accbdd9..96aa2380398 100644 --- a/src/cascadia/TerminalApp/TabManagement.cpp +++ b/src/cascadia/TerminalApp/TabManagement.cpp @@ -781,16 +781,6 @@ namespace winrt::TerminalApp::implementation // - pane: the pane to close. void TerminalPage::_HandleClosePaneRequested(std::shared_ptr pane) { - // GH#20187: Defensive guard. Callers should already filter out - // corpse panes (see _CloseFocusedPane), but in case a future - // caller passes a leaf pane whose content has already been torn - // down by a prior Pane::Close(), bail before we try to call - // GetTerminalArgsForPane on it. - if (pane->IsClosed()) - { - return; - } - // Build the list of actions to recreate the closed pane, // BuildStartupActions returns the "first" pane and the rest of // its actions are assuming that first pane has been created first. @@ -824,13 +814,18 @@ namespace winrt::TerminalApp::implementation if (const auto pane{ activeTab->GetActivePane() }) { - // GH#20187: When closePane is auto-repeated (or rapidly - // re-pressed), the second invocation can arrive while the - // parent's close animation is still running and - // Tab::_activePane still points at the already-closed - // leaf. Detect that "corpse" state here and bail before - // we try to call GetTerminalArgsForPane on a leaf whose - // content has been torn down. + // GH#20187: When the user closes a pane with a keyboard + // shortcut, focus moves off the closed pane while keys + // are still depressed. The corresponding KeyUp events + // arrive at TabRowControl, whose KeyUp is wired to + // TerminalPage::_KeyDownHandler (see TerminalPage.xaml). + // _KeyDownHandler can't tell it was invoked from KeyUp + // and dispatches the close action a second time. By + // then the parent's close animation in + // _CloseChildRoutine is still running, so + // Tab::_activePane still points at the now-closed leaf + // ("corpse"). Detect that here and bail before we + // dereference its null content in GetTerminalArgsForPane. if (pane->IsClosed()) { co_return;