diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp index 072f28c843..6e8e2e1727 100644 --- a/src/cascadia/TerminalApp/AppActionHandlers.cpp +++ b/src/cascadia/TerminalApp/AppActionHandlers.cpp @@ -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 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(), diff --git a/src/cascadia/TerminalApp/DebugTapConnection.cpp b/src/cascadia/TerminalApp/DebugTapConnection.cpp index 41416c3b98..d5d157529f 100644 --- 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 buffer) diff --git a/src/cascadia/TerminalApp/Jumplist.cpp b/src/cascadia/TerminalApp/Jumplist.cpp index e713514973..29996bd0b7 100644 --- a/src/cascadia/TerminalApp/Jumplist.cpp +++ b/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 diff --git a/src/cascadia/TerminalApp/SnippetsPaneContent.cpp b/src/cascadia/TerminalApp/SnippetsPaneContent.cpp index d2dd9fb03e..f7b8885015 100644 --- a/src/cascadia/TerminalApp/SnippetsPaneContent.cpp +++ b/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 - 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 index 3c34c5b3ec..e43fede75c 100644 --- a/src/cascadia/TerminalApp/TabManagement.cpp +++ b/src/cascadia/TerminalApp/TabManagement.cpp @@ -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 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 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) diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 37644fa296..927d2b4a34 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -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: // - - 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() }) { 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 index 84b80b10b3..22b73f89d8 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -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> _FindPackageAsync(hstring query); + static Windows::Foundation::IAsyncOperation> _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 index b652cc79e3..b97890d494 100644 --- 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 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() }; - 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(); + auto theme{ strong->_settings.GlobalSettings().CurrentTheme() }; + auto requestedTheme{ theme.RequestedTheme() }; + auto element{ sender.try_as() }; + while (element) + { + element.RequestedTheme(requestedTheme); + element = element.Parent().try_as(); + } } } }; diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 2cc35ce683..a1c541c908 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -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()) { diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp index 5dea2594e9..acaed85c49 100644 --- a/src/cascadia/WindowsTerminal/WindowEmperor.cpp +++ b/src/cascadia/WindowsTerminal/WindowEmperor.cpp @@ -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;