Use smart pointers in coroutines (#19752)

lhecker · DHowett · commit a0724f94b1c5 · 2026-01-21T15:38:11.000-06:00
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 a528141) Service-Card-Id: PVTI_lADOAF3p4s4AxadtzgjzPAU Service-Version: 1.23
diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp
@@ -898,7 +898,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
@@ -1451,6 +1454,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 };
@@ -1517,7 +1522,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(),
diff --git a/src/cascadia/TerminalApp/DebugTapConnection.cpp b/src/cascadia/TerminalApp/DebugTapConnection.cpp
@@ -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)
diff --git a/src/cascadia/TerminalApp/Jumplist.cpp b/src/cascadia/TerminalApp/Jumplist.cpp
@@ -78,6 +78,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
diff --git a/src/cascadia/TerminalApp/SnippetsPaneContent.cpp b/src/cascadia/TerminalApp/SnippetsPaneContent.cpp
@@ -47,13 +47,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)
diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp
@@ -403,12 +403,18 @@ namespace winrt::TerminalApp::implementation
     // - tab: the tab to remove
     winrt::Windows::Foundation::IAsyncAction TerminalPage::_HandleCloseTabRequested(winrt::TerminalApp::TabBase 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;
             }
@@ -719,10 +725,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;
             }
@@ -780,9 +790,13 @@ namespace winrt::TerminalApp::implementation
 
             if (const auto pane{ terminalTab->GetActivePane() })
             {
+                const auto weak = get_weak();
                 if (co_await _PaneConfirmCloseReadOnly(pane))
                 {
-                    _HandleClosePaneRequested(pane);
+                    if (const auto strong = weak.get())
+                    {
+                        _HandleClosePaneRequested(pane);
+                    }
                 }
             }
         }
@@ -840,9 +854,22 @@ namespace winrt::TerminalApp::implementation
     // - tabs - tabs to remove
     safe_void_coroutine TerminalPage::_RemoveTabs(const std::vector<winrt::TerminalApp::TabBase> 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:
@@ -919,18 +946,23 @@ namespace winrt::TerminalApp::implementation
 
     safe_void_coroutine TerminalPage::_OnTabPointerReleasedCloseTab(winrt::Microsoft::UI::Xaml::Controls::TabViewItem sender)
     {
+        // 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 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)
         {
             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())
         {
diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -2044,7 +2044,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)
@@ -3084,8 +3092,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:
@@ -3158,6 +3170,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
@@ -3175,7 +3193,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)
@@ -3255,6 +3278,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) {
@@ -4633,12 +4659,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();
 
diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h
@@ -512,7 +512,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);
@@ -528,7 +528,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);
diff --git a/src/cascadia/TerminalApp/TerminalWindow.cpp b/src/cascadia/TerminalApp/TerminalWindow.cpp
@@ -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>();
+                }
             }
         } };
 
diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp
diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp

Original file line number	Diff line number	Diff line change
`@@ -403,12 +403,18 @@ namespace winrt::TerminalApp::implementation`
`403`	`403`	`// - tab: the tab to remove`
`404`	`404`	`winrt::Windows::Foundation::IAsyncAction TerminalPage::_HandleCloseTabRequested(winrt::TerminalApp::TabBase tab)`
`405`	`405`	`{`
	`406`	`+ winrt::com_ptr<TerminalPage> strong;`
	`407`	`+`
`406`	`408`	`if (tab.ReadOnly())`
`407`	`409`	`{`
	`410`	`+ const auto weak = get_weak();`
	`411`	`+`
`408`	`412`	`auto warningResult = co_await _ShowCloseReadOnlyDialog();`
`409`	`413`
	`414`	`+ strong = weak.get();`
	`415`	`+`
`410`	`416`	`// If the user didn't explicitly click on close tab - leave`
`411`		`- if (warningResult != ContentDialogResult::Primary)`
	`417`	`+ if (!strong \|\| warningResult != ContentDialogResult::Primary)`
`412`	`418`	`{`
`413`	`419`	`co_return;`
`414`	`420`	`}`
`@@ -719,10 +725,14 @@ namespace winrt::TerminalApp::implementation`
`719`	`725`	`{`
`720`	`726`	`if (pane->ContainsReadOnly())`
`721`	`727`	`{`
	`728`	`+ const auto weak = get_weak();`
	`729`	`+`
`722`	`730`	`auto warningResult = co_await _ShowCloseReadOnlyDialog();`
`723`	`731`
	`732`	`+ const auto strong = weak.get();`
	`733`	`+`
`724`	`734`	`// If the user didn't explicitly click on close tab - leave`
`725`		`- if (warningResult != ContentDialogResult::Primary)`
	`735`	`+ if (!strong \|\| warningResult != ContentDialogResult::Primary)`
`726`	`736`	`{`
`727`	`737`	`co_return false;`
`728`	`738`	`}`
`@@ -780,9 +790,13 @@ namespace winrt::TerminalApp::implementation`
`780`	`790`
`781`	`791`	`if (const auto pane{ terminalTab->GetActivePane() })`
`782`	`792`	`{`
	`793`	`+ const auto weak = get_weak();`
`783`	`794`	`if (co_await _PaneConfirmCloseReadOnly(pane))`
`784`	`795`	`{`
`785`		`- _HandleClosePaneRequested(pane);`
	`796`	`+ if (const auto strong = weak.get())`
	`797`	`+ {`
	`798`	`+ _HandleClosePaneRequested(pane);`
	`799`	`+ }`
`786`	`800`	`}`
`787`	`801`	`}`
`788`	`802`	`}`
`@@ -840,9 +854,22 @@ namespace winrt::TerminalApp::implementation`
`840`	`854`	`// - tabs - tabs to remove`
`841`	`855`	`safe_void_coroutine TerminalPage::_RemoveTabs(const std::vector<winrt::TerminalApp::TabBase> tabs)`
`842`	`856`	`{`
	`857`	`+ const auto weak = get_weak();`
	`858`	`+`
`843`	`859`	`for (auto& tab : tabs)`
`844`	`860`	`{`
`845`		`- co_await _HandleCloseTabRequested(tab);`
	`861`	`+ winrt::Windows::Foundation::IAsyncAction action{ nullptr };`
	`862`	`+ if (const auto strong = weak.get())`
	`863`	`+ {`
	`864`	`+ action = _HandleCloseTabRequested(tab);`
	`865`	`+ }`
	`866`	`+`
	`867`	`+ if (!action)`
	`868`	`+ {`
	`869`	`+ co_return;`
	`870`	`+ }`
	`871`	`+`
	`872`	`+ co_await action;`
`846`	`873`	`}`
`847`	`874`	`}`
`848`	`875`	`// Method Description:`
`@@ -919,18 +946,23 @@ namespace winrt::TerminalApp::implementation`
`919`	`946`
`920`	`947`	`safe_void_coroutine TerminalPage::_OnTabPointerReleasedCloseTab(winrt::Microsoft::UI::Xaml::Controls::TabViewItem sender)`
`921`	`948`	`{`
	`949`	`+ // WinUI asynchronously updates its tab view items, so it may happen that we're given a`
	`950`	+ // `TabViewItem` that still contains a `TabBase` which has actually already been removed.
	`951`	`+ // First we must yield once, to flush out whatever TabView is currently doing.`
	`952`	`+ const auto weak = get_weak();`
	`953`	`+ co_await wil::resume_foreground(Dispatcher());`
	`954`	`+ const auto strong = weak.get();`
	`955`	`+ if (!strong)`
	`956`	`+ {`
	`957`	`+ co_return;`
	`958`	`+ }`
	`959`	`+`
`922`	`960`	`const auto tab = _GetTabByTabViewItem(sender);`
`923`	`961`	`if (!tab)`
`924`	`962`	`{`
`925`	`963`	`co_return;`
`926`	`964`	`}`
`927`	`965`
`928`		`- // WinUI asynchronously updates its tab view items, so it may happen that we're given a`
`929`		- // `TabViewItem` that still contains a `TabBase` which has actually already been removed.
`930`		`- // First we must yield once, to flush out whatever TabView is currently doing.`
`931`		`- const auto strong = get_strong();`
`932`		`- co_await wil::resume_foreground(Dispatcher());`
`933`		`-`
`934`	`966`	// `tab.Shutdown()` in `_RemoveTab()` sets the content to null = This checks if the tab is closed.
`935`	`967`	`if (tab.Content())`
`936`	`968`	`{`