Use smart pointers in coroutines (#19752)

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

Closes #19745

src/cascadia/TerminalApp/AppActionHandlers.cpp

@@ -903,7 +903,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
@@ -1456,6 +1459,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 };
@@ -1522,7 +1527,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(),

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)

src/cascadia/TerminalApp/Jumplist.cpp

@@ -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

src/cascadia/TerminalApp/SnippetsPaneContent.cpp

@@ -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)

src/cascadia/TerminalApp/TabManagement.cpp

@@ -396,12 +396,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;
             }
@@ -710,10 +716,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;
             }
@@ -771,9 +781,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);
+                    }
                 }
             }
         }
@@ -831,9 +845,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:
@@ -917,8 +944,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)

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -2207,7 +2207,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)
@@ -3205,8 +3213,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:
@@ -3263,6 +3275,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
@@ -3280,7 +3298,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)
@@ -3375,6 +3398,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) {
@@ -4736,12 +4762,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();
 

src/cascadia/TerminalApp/TerminalPage.h

@@ -526,7 +526,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);
@@ -542,7 +542,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);

src/cascadia/TerminalApp/TerminalWindow.cpp

@@ -323,6 +323,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.
@@ -336,7 +340,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,
@@ -353,7 +357,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
@@ -367,14 +371,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>();
+                }
             }
         } };
 

src/cascadia/TerminalControl/TermControl.cpp

@@ -3049,12 +3049,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();
         }
@@ -3063,7 +3068,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();
         }
@@ -3072,7 +3080,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();
         }
@@ -3134,6 +3145,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                     }
                 }
 
+                const auto strong = weak.get();
+                if (!strong)
+                {
+                    co_return;
+                }
+
                 std::wstring allPathsString;
                 for (auto& fullPath : fullPaths)
                 {
@@ -3476,9 +3493,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())
             {

src/cascadia/WindowsTerminal/WindowEmperor.cpp

@@ -861,6 +861,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;