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

Fix a crash when closing tabs (#18620)

Browse Source 
WinUI asynchronously updates its tab view items, so it may happen that
we're given a `TabViewItem` that still contains a `TabBase` which has
actually already been removed. Regressed in #15924.

Closes #18581

## Validation Steps Performed
* Close tabs rapidly with middle click
* No crash ✅

(cherry picked from commit 62e7f4bfade3b5dc655f1c8d8c0ceb49d89faf24)
Service-Card-Id: PVTI_lADOAF3p4s4AxadtzgXpjpM PVTI_lADOAF3p4s4AxadtzgXsRQM
Service-Version: 1.23
This commit is contained in:
Leonard Hecker 2025-02-24 10:31:32 -08:00 committed by Dustin L. Howett
parent a72531014c
commit 0d7e1293fa
4 changed files with 31 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
src/cascadia/TerminalApp/TabBase.cpp
View File

@ -32,6 +32,7 @@ namespace winrt::TerminalApp::implementation
{
ASSERT_UI_THREAD();
// NOTE: `TerminalPage::_HandleCloseTabRequested` relies on the content being null after this call.
Content(nullptr);
}

30
src/cascadia/TerminalApp/TabManagement.cpp
View File

@ -158,6 +158,8 @@ namespace winrt::TerminalApp::implementation
// Set this tab's icon to the icon from the content
_UpdateTabIcon(*newTabImpl);
// This is necessary, because WinUI does not have support for middle clicks.
// Its Tapped event doesn't provide the information what button was used either.
tabViewItem.PointerPressed({ this, &TerminalPage::_OnTabPointerPressed });
tabViewItem.PointerReleased({ this, &TerminalPage::_OnTabPointerReleased });
tabViewItem.PointerExited({ this, &TerminalPage::_OnTabPointerExited });
@ -903,19 +905,39 @@ namespace winrt::TerminalApp::implementation
if (_tabPointerMiddleButtonPressed && !eventArgs.GetCurrentPoint(nullptr).Properties().IsMiddleButtonPressed())
{
_tabPointerMiddleButtonPressed = false;
if (const auto tabViewItem{ sender.try_as<MUX::Controls::TabViewItem>() })
if (auto tabViewItem{ sender.try_as<MUX::Controls::TabViewItem>() })
{
tabViewItem.ReleasePointerCapture(eventArgs.Pointer());
auto tab = _GetTabByTabViewItem(tabViewItem);
if (!_tabPointerMiddleButtonExited && tab)
if (!_tabPointerMiddleButtonExited)
{
_HandleCloseTabRequested(tab);
_OnTabPointerReleasedCloseTab(std::move(tabViewItem));
}
}
eventArgs.Handled(true);
}
}
safe_void_coroutine TerminalPage::_OnTabPointerReleasedCloseTab(winrt::Microsoft::UI::Xaml::Controls::TabViewItem sender)
{
const auto tab = _GetTabByTabViewItem(sender);
if (!tab)
{
co_return;
}
// WinUI asynchronously updates its tab view items, so it may happen that we're given a
// `TabViewItem` that still contains a `TabBase` which has actually already been removed.
// First we must yield once, to flush out whatever TabView is currently doing.
const auto strong = get_strong();
co_await wil::resume_foreground(Dispatcher());
// `tab.Shutdown()` in `_RemoveTab()` sets the content to null = This checks if the tab is closed.
if (tab.Content())
{
_HandleCloseTabRequested(tab);
}
}
// Method Description:
// - Tracking pointer state for tab remove
// Arguments:

1
src/cascadia/TerminalApp/TerminalPage.h
View File

@ -433,6 +433,7 @@ namespace winrt::TerminalApp::implementation
bool _tabPointerMiddleButtonExited{ false };
void _OnTabPointerPressed(const IInspectable& sender, const Windows::UI::Xaml::Input::PointerRoutedEventArgs& eventArgs);
void _OnTabPointerReleased(const IInspectable& sender, const Windows::UI::Xaml::Input::PointerRoutedEventArgs& eventArgs);
safe_void_coroutine _OnTabPointerReleasedCloseTab(winrt::Microsoft::UI::Xaml::Controls::TabViewItem sender);
void _OnTabPointerEntered(const IInspectable& sender, const Windows::UI::Xaml::Input::PointerRoutedEventArgs& eventArgs);
void _OnTabPointerExited(const IInspectable& sender, const Windows::UI::Xaml::Input::PointerRoutedEventArgs& eventArgs);

3
src/cascadia/TerminalApp/TerminalTab.cpp
View File

@ -851,6 +851,9 @@ namespace winrt::TerminalApp::implementation
{
ASSERT_UI_THREAD();
// Don't forget to call the overridden function. :)
TabBase::Shutdown();
if (_rootPane)
{
_rootPane->Shutdown();