diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp index e81a6c1d80..48ed69822a 100644 --- a/src/cascadia/TerminalApp/AppActionHandlers.cpp +++ b/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 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(), diff --git a/src/cascadia/TerminalApp/DebugTapConnection.cpp b/src/cascadia/TerminalApp/DebugTapConnection.cpp index 7cfa19b5a0..9c545baf15 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 8869468aae..21e31fb6dd 100644 --- a/src/cascadia/TerminalApp/TabManagement.cpp +++ b/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 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 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) diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 5d9b2b1ba1..b8f0dab41a 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/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: // - - 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 10b58e175d..4b48cc0e9d 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/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> _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 27a573e320..e4d451a674 100644 --- a/src/cascadia/TerminalApp/TerminalWindow.cpp +++ b/src/cascadia/TerminalApp/TerminalWindow.cpp @@ -323,6 +323,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. @@ -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() }; - 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 47a70a6b15..d3570ff1b5 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/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()) { diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp index 750a1337a0..64fb25e02f 100644 --- a/src/cascadia/WindowsTerminal/WindowEmperor.cpp +++ b/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;