Crash in main when writing settings to disk twice

[PankajBhojwani]

Windows Terminal version (or Windows build number)

main

Other Software

N/A

Steps to reproduce

1. Open SUI
2. Navigate to any profile
3. Change any setting
4. Hit 'Save'
5. Hit 'Save' again
6. Crash

Alternatively:

1. Open SUI
2. Add a new profile
3. Hit 'Save'
4. Hit 'Save' again
5. Crash

Expected Behavior

No crash

Actual Behavior

Terminal crashes. The crash seems to happen in the Model::DefaultTerminal::Current(_currentDefaultTerminal); call in CascadiaSettings::WriteSettingsToDisk(), added in #9907

[DHowett]

Looks like a @miniksa crash 😄

[miniksa]

When I leave a breakpoint on the Model::DefaultTerminal::Current(_currentDefaultTerminal) setting under the debugger... this never happens. I think there's a race in refreshing the model between hitting Save that could be resulting in a null....

[zadjii-msft]

Okay so I just tried this out and I could not get it to happen again. Maybe this fixed itself? I'm testing Version: 1.9.1391.0 now.

I was assuming that this was caused by the DelegationBase.clsid being technically uninitialized, and only hitting in Release. That would at least make sense to me. Though, that doesn't strike me as something that should crash - it should just gracefully RETURN_IF_FAILED

The original call stack I had for this crash was:

0:000> k
 # Child-SP          RetAddr               Call Site
00 (Inline Function) --------`--------     Microsoft_Terminal_Settings_Model!DelegationConfig::s_SetDefaultConsoleById+0x1f [E:\BA\242\s\src\propslib\DelegationConfig.cpp @ 244] 
01 (Inline Function) --------`--------     Microsoft_Terminal_Settings_Model!DelegationConfig::s_SetDefaultByPackage+0x1f [E:\BA\242\s\src\propslib\DelegationConfig.cpp @ 254] 
02 0000004f`515dc0f0 00007ff8`25b483b4     Microsoft_Terminal_Settings_Model!winrt::Microsoft::Terminal::Settings::Model::implementation::DefaultTerminal::Current+0x31 [E:\BA\242\s\src\cascadia\TerminalSettingsModel\DefaultTerminal.cpp @ 89] 
03 (Inline Function) --------`--------     Microsoft_Terminal_Settings_Model!winrt::Microsoft::Terminal::Settings::Model::DefaultTerminal::Current+0x5 [E:\BA\242\s\src\cascadia\TerminalSettingsModel\Generated Files\DefaultTerminal.g.cpp @ 15] 
04 0000004f`515dc130 00007ff8`25afe0a9     Microsoft_Terminal_Settings_Model!winrt::Microsoft::Terminal::Settings::Model::implementation::CascadiaSettings::WriteSettingsToDisk+0x4e4 [E:\BA\242\s\src\cascadia\TerminalSettingsModel\CascadiaSettingsSerialization.cpp @ 1304] 
05 0000004f`515dc3f0 00007fff`fd3ddf31     Microsoft_Terminal_Settings_Model!winrt::impl::produce<winrt::Microsoft::Terminal::Settings::Model::implementation::CascadiaSettings,winrt::Microsoft::Terminal::Settings::Model::ICascadiaSettings>::WriteSettingsToDisk+0x19 [E:\BA\242\s\src\cascadia\TerminalSettingsModel\Generated Files\winrt\Microsoft.Terminal.Settings.Model.h @ 3449] 
06 (Inline Function) --------`--------     Microsoft_Terminal_Settings_Editor!winrt::impl::consume_Microsoft_Terminal_Settings_Model_ICascadiaSettings<winrt::Microsoft::Terminal::Settings::Model::ICascadiaSettings>::WriteSettingsToDisk+0x14 [E:\BA\242\s\src\cascadia\TerminalSettingsEditor\Generated Files\winrt\Microsoft.Terminal.Settings.Model.h @ 502] 
07 (Inline Function) --------`--------     Microsoft_Terminal_Settings_Editor!winrt::Microsoft::Terminal::Settings::Editor::implementation::MainPage::SaveButton_Click+0x14 [E:\BA\242\s\src\cascadia\TerminalSettingsEditor\MainPage.cpp @ 348] 
08 (Inline Function) --------`--------     Microsoft_Terminal_Settings_Editor!winrt::Microsoft::Terminal::Settings::Editor::implementation::MainPageT<winrt::Microsoft::Terminal::Settings::Editor::implementation::MainPage>::Connect::__l9::<lambda_be15dafd2ad1c7807964ddd9d83a6bae>::operator()+0x4c [E:\BA\242\s\src\cascadia\TerminalSettingsEditor\Generated Files\MainPage.xaml.g.hpp @ 105] 
09 0000004f`515dc420 00007ff8`26fc8aab     Microsoft_Terminal_Settings_Editor!winrt::impl::delegate<winrt::Windows::UI::Xaml::RoutedEventHandler,<lambda_be15dafd2ad1c7807964ddd9d83a6bae> >::Invoke+0x61 [E:\BA\242\s\src\cascadia\TerminalSettingsEditor\Generated Files\winrt\Windows.UI.Xaml.h @ 4670] 
0a 0000004f`515dc470 00007ff8`2708252c     Windows_UI_Xaml!DirectUI::CRoutedEventSourceBase<DirectUI::IUntypedEventSource,Windows::UI::Xaml::IRoutedEventHandler,IInspectable,Windows::UI::Xaml::IRoutedEventArgs>::Raise+0xeb [onecoreuap\Windows\dxaml\xcp\dxaml\lib\JoltClasses.h @ 1041] 
0b 0000004f`515dc520 00007ff8`270828ff     Windows_UI_Xaml!DirectUI::ButtonBase::OnClick+0x9c

EDIT:

Interesting. In the original stack, CascadiaSettings::_currentDefaultTerminal is null. So it's not that there's half a GUID - it's literally just garbage data that managed to survive that long. How do we get to CascadiaSettings::WriteSettingsToDisk with a null _currentDefaultTerminal?

[zadjii-msft]

Nevermind. Found the specific repro. You absolutely must be on not the launch page. So yea, saving on a cmd profile will hit it. ^{Yes the OP had that info, I'm just daft}

My guess as to what's happening - CascadiaSettings::RefreshDefaultTerminals only gets called when we open the Launch page. So that's the only time that _currentDefaultTerminal is initialized. Otherwise, it's left as null, and we'll crash on the second save, trying to CascadiaSettings::WriteSettingsToDisk the null _currentDefaultTerminal. We can totally fix that.

[ghost]

🎉This issue was addressed in #10148, which has now been successfully released as Windows Terminal Preview v1.9.1445.0.:tada:

Handy links:

• Release Notes
• Store Download