Skip to content

Commit

Permalink
Fix a crash during settings update (#17751)
Browse files Browse the repository at this point in the history
* 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. 🚫
  • Loading branch information
lhecker authored Aug 20, 2024
1 parent 60ac45c commit e0dae59
Showing 1 changed file with 11 additions and 2 deletions.
13 changes: 11 additions & 2 deletions src/cascadia/TerminalApp/TerminalWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -767,13 +767,22 @@ namespace winrt::TerminalApp::implementation
// definitely not on OUR UI thread.
winrt::fire_and_forget TerminalWindow::UpdateSettings(winrt::TerminalApp::SettingsLoadEventArgs args)
{
_settings = args.NewSettings();
// GH#17620: We have a bug somewhere where a window doesn't get unregistered from the window list.
// This causes UpdateSettings calls where the thread dispatcher is already null.
const auto dispatcher = _root->Dispatcher();
if (!dispatcher)
{
co_return;
}

const auto weakThis{ get_weak() };
co_await wil::resume_foreground(_root->Dispatcher());
co_await wil::resume_foreground(dispatcher);

// Back on our UI thread...
if (auto logic{ weakThis.get() })
{
_settings = args.NewSettings();

// Update the settings in TerminalPage
// We're on our UI thread right now, so this is safe
_root->SetSettings(_settings, true);
Expand Down

0 comments on commit e0dae59

Please sign in to comment.