Skip to content

Commit 42ab9bb

Browse files
lheckerDHowett
authored andcommitted
Fix a crash during settings update (#17751)
* Adds a check whether the thread dispatcher is already null. (See code comments.) * Moves the `_settings` to only happen on the UI thread. Anything else wouldn't be thread safe. Closes #17620 ## Validation Steps Performed Not reproducible. 🚫 (cherry picked from commit e0dae59) Service-Card-Id: PVTI_lADOAF3p4s4AmhmszgSGBis Service-Version: 1.21
1 parent cb8a3f6 commit 42ab9bb

File tree

1 file changed

+11
-2
lines changed

1 file changed

+11
-2
lines changed

src/cascadia/TerminalApp/TerminalWindow.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -767,13 +767,22 @@ namespace winrt::TerminalApp::implementation
767767
// definitely not on OUR UI thread.
768768
winrt::fire_and_forget TerminalWindow::UpdateSettings(winrt::TerminalApp::SettingsLoadEventArgs args)
769769
{
770-
_settings = args.NewSettings();
770+
// GH#17620: We have a bug somewhere where a window doesn't get unregistered from the window list.
771+
// This causes UpdateSettings calls where the thread dispatcher is already null.
772+
const auto dispatcher = _root->Dispatcher();
773+
if (!dispatcher)
774+
{
775+
co_return;
776+
}
771777

772778
const auto weakThis{ get_weak() };
773-
co_await wil::resume_foreground(_root->Dispatcher());
779+
co_await wil::resume_foreground(dispatcher);
780+
774781
// Back on our UI thread...
775782
if (auto logic{ weakThis.get() })
776783
{
784+
_settings = args.NewSettings();
785+
777786
// Update the settings in TerminalPage
778787
// We're on our UI thread right now, so this is safe
779788
_root->SetSettings(_settings, true);

0 commit comments

Comments
 (0)