microsoft/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-02-04 03:05:08 -06:00
Code Issues Packages Projects Releases 138 Wiki Activity

Use smart pointers in coroutines (#19752)

Browse Source 
After the coroutines resume, `this` may have already been destroyed.
So, this PR adds proper use of `get_weak/strong`.

Closes #19745

(cherry picked from commit a528141fe6cbf10a3a9ce3f8680eb70d3a27df20)
Service-Card-Id: PVTI_lADOAF3p4s4BBcTlzgjzPAI
Service-Version: 1.24
This commit is contained in:
Leonard Hecker 2026-01-16 22:43:30 +01:00 committed by Dustin L. Howett
parent 633cd6e9dc
commit 51fd044bd2
10 changed files with 151 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
src/cascadia/TerminalApp/AppActionHandlers.cpp
View File

@ -900,7 +900,10 @@ namespace winrt::TerminalApp::implementation
co_return;
}
// Hop to the BG thread
// ShellExecuteExW may block, so do it on a background thread.
//
// NOTE: All remaining code of this function doesn't touch `this`, so we don't need weak/strong_ref.
// NOTE NOTE: Don't touch `this` when you make changes here.
co_await winrt::resume_background();
// This will get us the correct exe for dev/preview/release. If you
@ -1453,6 +1456,8 @@ namespace winrt::TerminalApp::implementation
safe_void_coroutine TerminalPage::_doHandleSuggestions(SuggestionsArgs realArgs)
{
const auto weak = get_weak();
const auto dispatcher = Dispatcher();
const auto source = realArgs.Source();
std::vector<Command> commandsCollection;
Control::CommandHistoryContext context{ nullptr };
@ -1519,7 +1524,12 @@ namespace winrt::TerminalApp::implementation
}
}
co_await wil::resume_foreground(Dispatcher());
co_await wil::resume_foreground(dispatcher);
const auto strong = weak.get();
if (!strong)
{
co_return;
}
// Open the palette with all these commands in it.
_OpenSuggestions(_GetActiveControl(),

8
src/cascadia/TerminalApp/DebugTapConnection.cpp
View File

@ -34,9 +34,15 @@ namespace winrt::Microsoft::TerminalApp::implementation
// before actually starting the connection to the client app. This
// will ensure both controls are initialized before the client app
// is.
const auto weak = get_weak();
co_await winrt::resume_background();
_pairedTap->_start.wait();
const auto strong = weak.get();
if (!strong)
{
co_return;
}
_pairedTap->_start.wait();
_wrappedConnection.Start();
}
void WriteInput(const winrt::array_view<const char16_t> buffer)

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

@ -43,6 +43,9 @@ safe_void_coroutine Jumplist::UpdateJumplist(const CascadiaSettings& settings) n
// make sure to capture the settings _before_ the co_await
const auto strongSettings = settings;
// Explorer APIs may block, so do it on a background thread.
//
// NOTE: Jumplist has no members, so we don't need to hold onto `this` here.
co_await winrt::resume_background();
try

11
src/cascadia/TerminalApp/SnippetsPaneContent.cpp
View File

@ -48,13 +48,22 @@ namespace winrt::TerminalApp::implementation
{
_settings = settings;
const auto dispatcher = Dispatcher();
const auto weak = get_weak();
// You'd think that `FilterToSendInput(queryString)` would work. It
// doesn't! That uses the queryString as the current command the user
// has typed, then relies on the suggestions UI to _also_ filter with that
// string.
const auto tasks = co_await _settings.GlobalSettings().ActionMap().FilterToSnippets(winrt::hstring{}, winrt::hstring{}); // IVector<Model::Command>
co_await wil::resume_foreground(Dispatcher());
co_await wil::resume_foreground(dispatcher);
const auto strong = weak.get();
if (!strong)
{
co_return;
}
_allTasks.Clear();
for (const auto& t : tasks)

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

@ -397,12 +397,18 @@ namespace winrt::TerminalApp::implementation
// - tab: the tab to remove
winrt::Windows::Foundation::IAsyncAction TerminalPage::_HandleCloseTabRequested(winrt::TerminalApp::Tab tab)
{
winrt::com_ptr<TerminalPage> strong;
if (tab.ReadOnly())
{
const auto weak = get_weak();
auto warningResult = co_await _ShowCloseReadOnlyDialog();
strong = weak.get();
// If the user didn't explicitly click on close tab - leave
if (warningResult != ContentDialogResult::Primary)
if (!strong || warningResult != ContentDialogResult::Primary)
{
co_return;
}
@ -713,10 +719,14 @@ namespace winrt::TerminalApp::implementation
{
if (pane->ContainsReadOnly())
{
const auto weak = get_weak();
auto warningResult = co_await _ShowCloseReadOnlyDialog();
const auto strong = weak.get();
// If the user didn't explicitly click on close tab - leave
if (warningResult != ContentDialogResult::Primary)
if (!strong || warningResult != ContentDialogResult::Primary)
{
co_return false;
}
@ -774,9 +784,13 @@ namespace winrt::TerminalApp::implementation
if (const auto pane{ activeTab->GetActivePane() })
{
const auto weak = get_weak();
if (co_await _PaneConfirmCloseReadOnly(pane))
{
_HandleClosePaneRequested(pane);
if (const auto strong = weak.get())
{
_HandleClosePaneRequested(pane);
}
}
}
}
@ -834,9 +848,22 @@ namespace winrt::TerminalApp::implementation
// - tabs - tabs to remove
safe_void_coroutine TerminalPage::_RemoveTabs(const std::vector<winrt::TerminalApp::Tab> tabs)
{
const auto weak = get_weak();
for (auto& tab : tabs)
{
co_await _HandleCloseTabRequested(tab);
winrt::Windows::Foundation::IAsyncAction action{ nullptr };
if (const auto strong = weak.get())
{
action = _HandleCloseTabRequested(tab);
}
if (!action)
{
co_return;
}
co_await action;
}
}
// Method Description:
@ -920,8 +947,13 @@ namespace winrt::TerminalApp::implementation
// WinUI asynchronously updates its tab view items, so it may happen that we're given a
// `TabViewItem` that still contains a `Tab` which has actually already been removed.
// First we must yield once, to flush out whatever TabView is currently doing.
const auto strong = get_strong();
const auto weak = get_weak();
co_await wil::resume_foreground(Dispatcher());
const auto strong = weak.get();
if (!strong)
{
co_return;
}
const auto tab = _GetTabByTabViewItem(sender);
if (!tab)

38
src/cascadia/TerminalApp/TerminalPage.cpp
View File

@ -2216,7 +2216,15 @@ namespace winrt::TerminalApp::implementation
if (!_displayingCloseDialog)
{
_displayingCloseDialog = true;
const auto weak = get_weak();
auto warningResult = co_await _ShowQuitDialog();
const auto strong = weak.get();
if (!strong)
{
co_return;
}
_displayingCloseDialog = false;
if (warningResult != ContentDialogResult::Primary)
@ -3191,8 +3199,12 @@ namespace winrt::TerminalApp::implementation
// - eventArgs: the arguments specifying how to set the progress indicator
safe_void_coroutine TerminalPage::_SetTaskbarProgressHandler(const IInspectable /*sender*/, const IInspectable /*eventArgs*/)
{
const auto weak = get_weak();
co_await wil::resume_foreground(Dispatcher());
SetTaskbarProgress.raise(*this, nullptr);
if (const auto strong = weak.get())
{
SetTaskbarProgress.raise(*this, nullptr);
}
}
// Method Description:
@ -3249,6 +3261,12 @@ namespace winrt::TerminalApp::implementation
{
co_return;
}
const auto weak = get_weak();
const auto dispatcher = Dispatcher();
// All of the code until resume_foreground is static and
// doesn't touch `this`, so we don't need weak/strong_ref.
co_await winrt::resume_background();
// no packages were found, nothing to suggest
@ -3266,7 +3284,12 @@ namespace winrt::TerminalApp::implementation
suggestions.emplace_back(fmt::format(FMT_COMPILE(L"winget install --id {} -s winget"), pkg.CatalogPackage().Id()));
}
co_await wil::resume_foreground(Dispatcher());
co_await wil::resume_foreground(dispatcher);
const auto strong = weak.get();
if (!strong)
{
co_return;
}
auto term = _GetActiveControl();
if (!term)
@ -3361,6 +3384,9 @@ namespace winrt::TerminalApp::implementation
// UI) thread. This is IMPORTANT, because the Windows.Storage API's
// (used for retrieving the path to the file) will crash on the UI
// thread, because the main thread is a STA.
//
// NOTE: All remaining code of this function doesn't touch `this`, so we don't need weak/strong_ref.
// NOTE NOTE: Don't touch `this` when you make changes here.
co_await winrt::resume_background();
auto openFile = [](const auto& filePath) {
@ -4719,12 +4745,18 @@ namespace winrt::TerminalApp::implementation
// - sender: the ICoreState instance containing the connection state
// Return Value:
// - <none>
safe_void_coroutine TerminalPage::_ConnectionStateChangedHandler(const IInspectable& sender, const IInspectable& /*args*/) const
safe_void_coroutine TerminalPage::_ConnectionStateChangedHandler(const IInspectable& sender, const IInspectable& /*args*/)
{
if (const auto coreState{ sender.try_as<winrt::Microsoft::Terminal::Control::ICoreState>() })
{
const auto newConnectionState = coreState.ConnectionState();
const auto weak = get_weak();
co_await wil::resume_foreground(Dispatcher());
const auto strong = weak.get();
if (!strong)
{
co_return;
}
_adjustProcessPriorityThrottled->Run();

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

@ -520,7 +520,7 @@ namespace winrt::TerminalApp::implementation
const winrt::Microsoft::Terminal::Settings::Model::Profile& profile);
void _OpenElevatedWT(winrt::Microsoft::Terminal::Settings::Model::NewTerminalArgs newTerminalArgs);
safe_void_coroutine _ConnectionStateChangedHandler(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::Foundation::IInspectable& args) const;
safe_void_coroutine _ConnectionStateChangedHandler(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::Foundation::IInspectable& args);
void _CloseOnExitInfoDismissHandler(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::Foundation::IInspectable& args) const;
void _KeyboardServiceWarningInfoDismissHandler(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::Foundation::IInspectable& args) const;
static bool _IsMessageDismissed(const winrt::Microsoft::Terminal::Settings::Model::InfoBarMessage& message);
@ -536,7 +536,7 @@ namespace winrt::TerminalApp::implementation
void _ShowWindowChangedHandler(const IInspectable sender, const winrt::Microsoft::Terminal::Control::ShowWindowArgs args);
Windows::Foundation::IAsyncAction _SearchMissingCommandHandler(const IInspectable sender, const winrt::Microsoft::Terminal::Control::SearchMissingCommandEventArgs args);
Windows::Foundation::IAsyncOperation<Windows::Foundation::Collections::IVectorView<winrt::Microsoft::Management::Deployment::MatchResult>> _FindPackageAsync(hstring query);
static Windows::Foundation::IAsyncOperation<Windows::Foundation::Collections::IVectorView<winrt::Microsoft::Management::Deployment::MatchResult>> _FindPackageAsync(hstring query);
void _WindowSizeChanged(const IInspectable sender, const winrt::Microsoft::Terminal::Control::WindowSizeChangedEventArgs args);
void _windowPropertyChanged(const IInspectable& sender, const winrt::Windows::UI::Xaml::Data::PropertyChangedEventArgs& args);

25
src/cascadia/TerminalApp/TerminalWindow.cpp
View File

@ -322,6 +322,10 @@ namespace winrt::TerminalApp::implementation
// - an IAsyncOperation with the dialog result
winrt::Windows::Foundation::IAsyncOperation<ContentDialogResult> TerminalWindow::ShowDialog(winrt::WUX::Controls::ContentDialog dialog)
{
const auto weak = get_weak();
const auto dispatcher = _root->Dispatcher();
const auto root = _root->XamlRoot();
// As mentioned on s_activeDialog, dismissing the active dialog is necessary.
// We repeat it a few times in case the resume_foreground failed to work,
// but I found that one iteration will always be enough in practice.
@ -335,7 +339,7 @@ namespace winrt::TerminalApp::implementation
s_activeDialog.Hide();
// Wait for the current dialog to be hidden.
co_await wil::resume_foreground(_root->Dispatcher(), CoreDispatcherPriority::Low);
co_await wil::resume_foreground(dispatcher, CoreDispatcherPriority::Low);
}
// If two sources call ShowDialog() simultaneously, it may happen that both enter the above loop,
@ -352,7 +356,7 @@ namespace winrt::TerminalApp::implementation
// IMPORTANT: This is necessary as documented in the ContentDialog MSDN docs.
// Since we're hosting the dialog in a Xaml island, we need to connect it to the
// xaml tree somehow.
dialog.XamlRoot(_root->XamlRoot());
dialog.XamlRoot(root);
// IMPORTANT: Set the requested theme of the dialog, because the
// PopupRoot isn't directly in the Xaml tree of our root. So the dialog
@ -366,14 +370,17 @@ namespace winrt::TerminalApp::implementation
// theme on each element up to the root. We're relying a bit on Xaml's implementation
// details here, but it does have the desired effect.
// It's not enough to set the theme on the dialog alone.
auto themingLambda{ [this](const Windows::Foundation::IInspectable& sender, const RoutedEventArgs&) {
auto theme{ _settings.GlobalSettings().CurrentTheme() };
auto requestedTheme{ theme.RequestedTheme() };
auto element{ sender.try_as<winrt::Windows::UI::Xaml::FrameworkElement>() };
while (element)
auto themingLambda{ [weak](const Windows::Foundation::IInspectable& sender, const RoutedEventArgs&) {
if (const auto strong = weak.get())
{
element.RequestedTheme(requestedTheme);
element = element.Parent().try_as<winrt::Windows::UI::Xaml::FrameworkElement>();
auto theme{ strong->_settings.GlobalSettings().CurrentTheme() };
auto requestedTheme{ theme.RequestedTheme() };
auto element{ sender.try_as<winrt::Windows::UI::Xaml::FrameworkElement>() };
while (element)
{
element.RequestedTheme(requestedTheme);
element = element.Parent().try_as<winrt::Windows::UI::Xaml::FrameworkElement>();
}
}
} };

30
src/cascadia/TerminalControl/TermControl.cpp
View File

@ -3159,12 +3159,17 @@ namespace winrt::Microsoft::Terminal::Control::implementation
co_return;
}
const auto weak = get_weak();
if (e.DataView().Contains(StandardDataFormats::ApplicationLink()))
{
try
{
auto link{ co_await e.DataView().GetApplicationLinkAsync() };
_pasteTextWithBroadcast(link.AbsoluteUri());
if (const auto strong = weak.get())
{
_pasteTextWithBroadcast(link.AbsoluteUri());
}
}
CATCH_LOG();
}
@ -3173,7 +3178,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
try
{
auto link{ co_await e.DataView().GetWebLinkAsync() };
_pasteTextWithBroadcast(link.AbsoluteUri());
if (const auto strong = weak.get())
{
_pasteTextWithBroadcast(link.AbsoluteUri());
}
}
CATCH_LOG();
}
@ -3182,7 +3190,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
try
{
auto text{ co_await e.DataView().GetTextAsync() };
_pasteTextWithBroadcast(text);
if (const auto strong = weak.get())
{
_pasteTextWithBroadcast(text);
}
}
CATCH_LOG();
}
@ -3244,6 +3255,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}
}
const auto strong = weak.get();
if (!strong)
{
co_return;
}
std::wstring allPathsString;
for (auto& fullPath : fullPaths)
{
@ -3586,9 +3603,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation
safe_void_coroutine TermControl::_updateSelectionMarkers(IInspectable /*sender*/, Control::UpdateSelectionMarkersEventArgs args)
{
if (!args)
{
co_return;
}
auto weakThis{ get_weak() };
co_await resume_foreground(Dispatcher());
if (weakThis.get() && args)
if (const auto strong = weakThis.get())
{
if (_core.HasSelection() && !args.ClearMarkers())
{

3
src/cascadia/WindowsTerminal/WindowEmperor.cpp
View File

@ -860,6 +860,9 @@ safe_void_coroutine WindowEmperor::_showMessageBox(winrt::hstring message, bool
// We must yield to a background thread, because MessageBoxW() is a blocking call, and we can't
// block the main thread. That would prevent us from servicing WM_COPYDATA messages and similar.
//
// NOTE: All remaining code of this function doesn't touch `this`, so we don't need weak/strong_ref.
// NOTE NOTE: Don't touch `this` when you make changes here.
co_await winrt::resume_background();
const auto messageTitle = error ? IDS_ERROR_DIALOG_TITLE : IDS_HELP_DIALOG_TITLE;