Windows Terminal crashes when emptying settings.json with settings window open #18105

@zadjii-msft commented on GitHub (Aug 5, 2022):

I'll triage this as a separate bug for now, but cross my fingers and hope that it's the root cause of #12333. dmps loading now......

@zadjii-msft commented on GitHub (Aug 5, 2022): I'll triage this as a separate bug for now, but cross my fingers and hope that it's the root cause of #12333. dmps loading now......

claunia commented

@ashemedai commented on GitHub (Aug 5, 2022):

If need be, I created another feedback at https://aka.ms/AAhmblm this morning since my original didn't appear for over 12-13 hours under My Feedback.

@ashemedai commented on GitHub (Aug 5, 2022): If need be, I created another feedback at https://aka.ms/AAhmblm this morning since my original didn't appear for over 12-13 hours under My Feedback.

claunia commented

@zadjii-msft commented on GitHub (Aug 5, 2022):

Hmm. Coroutines are weird, but this looks like the error it's trying to report is "The operation attempted to access data outside the valid range..".

Interesting bit of the stack is:

2e (Inline Function) --------`--------     rpcrt4!Ndr64pSendReceive+0x2f [minio\rpc\ndr64\cltcall.cxx @ 184] 
2f 000000aa`24d9dab0 00007ffc`e6abc538     rpcrt4!NdrpClientCall3+0x3a4 [minio\rpc\ndr64\cltcall.cxx @ 328] 
30 000000aa`24d9de20 00007ffc`e6bcf642     combase!ObjectStublessClient+0x138 [onecore\com\combase\ndr\ndrole\amd64\stblsclt.cxx @ 369] 
31 000000aa`24d9e1b0 00007ffc`613f652d     combase!ObjectStubless+0x42 [onecore\com\combase\ndr\ndrole\amd64\stubless.asm @ 176] 
32 000000aa`24d9e200 00007ffc`613a8d24     TerminalApp!winrt::impl::consume_Windows_ApplicationModel_IStartupTask<winrt::Windows::ApplicationModel::IStartupTask>::State+0x2d [C:\a\_work\1\s\src\cascadia\TerminalApp\Generated Files\winrt\Windows.ApplicationModel.h @ 1077] 
33 000000aa`24d9e240 00007ffc`613a4d71     TerminalApp!winrt::TerminalApp::implementation::AppLogic::_ApplyStartupTaskStateChange$_ResumeCoro$1+0x1c4 [C:\a\_work\1\s\src\cascadia\TerminalApp\AppLogic.cpp @ 1004] 
34 (Inline Function) --------`--------     TerminalApp!std::experimental::coroutine_handle<void>::resume+0x9

Kinda looks like

        const auto task = co_await StartupTask::GetAsync(StartupTaskName);

        switch (task.State())
        {

Like, task was null?

Naw, this is in xaml. There's a stowed xaml exception:

    Windows_UI_Xaml!CUIElement::Measure+0x4bd  (7ffccced5dfd) [onecoreuap\windows\dxaml\xcp\core\core\elements\uielement.cpp @ 4063]
    Windows_UI_Xaml!CUIElement::Measure+0x4bd  (7ffccced5dfd) [onecoreuap\windows\dxaml\xcp\core\core\elements\uielement.cpp @ 4063]
    Windows_UI_Xaml!CUIElement::Measure+0x4bd  (7ffccced5dfd) [onecoreuap\windows\dxaml\xcp\core\core\elements\uielement.cpp @ 4063]
    Windows_UI_Xaml!CLayoutManager::UpdateLayout+0x107  (7ffcccecc987) [onecoreuap\windows\dxaml\xcp\core\layout\layoutmanager.cpp @ 279]
    Windows_UI_Xaml!CUIElement::UpdateLayout+0x3d  (7ffccce3d309) [onecoreuap\windows\dxaml\xcp\core\core\elements\uielement.cpp @ 3761]
    Windows_UI_Xaml!DirectUI::UIElementGenerated::UpdateLayout+0x47  (7ffccce3cac7) [onecoreuap\windows\dxaml\xcp\dxaml\lib\winrtgeneratedclasses\uielement.g.cpp @ 4804]
    Microsoft_UI_Xaml!winrt::impl::consume_Windows_UI_Xaml_IUIElement<winrt::Microsoft::UI::Xaml::Controls::ItemsRepeater>::UpdateLayout+0x53  (7ffc5427ebd3) [C:\a\_work\1\s\BuildOutput\Intermediates\x64\Microsoft.UI.Xaml\obj\Generated Files\winrt\Windows.UI.Xaml.h @ 3021]
    Microsoft_UI_Xaml!NavigationView::OnMenuItemsSourceCollectionChanged+0x24  (7ffc543041b4) [C:\a\_work\1\s\dev\NavigationView\NavigationView.cpp @ 3251]
    Microsoft_UI_Xaml!winrt::impl::delegate<winrt::Windows::UI::Xaml::SizeChangedEventHandler,<lambda_778e218533f9d63cc7aafcdd2f5bac52> >::Invoke+0x3b  (7ffc542cd5db) [C:\a\_work\1\s\BuildOutput\Intermediates\x64\Microsoft.UI.Xaml\obj\Generated Files\winrt\Windows.Foundation.h @ 894]
    Microsoft_UI_Xaml!event_base<event_source<winrt::Windows::UI::Xaml::Interop::NotifyCollectionChangedEventHandler>,winrt::Windows::UI::Xaml::Interop::NotifyCollectionChangedEventHandler,tracker_ref<winrt::Windows::UI::Xaml::Interop::NotifyCollectionChangedEventHandler,0,IUnknown *> >::operator  (7ffc5421fd87) [C:\a\_work\1\s\dev\inc\event.h @ 77]
    Microsoft_UI_Xaml!InspectingDataSource::OnVectorChanged+0xe4  (7ffc541d4294) [C:\a\_work\1\s\dev\Repeater\InspectingDataSource.cpp @ 260]
    Microsoft_UI_Xaml!winrt::impl::delegate<winrt::Windows::Foundation::TypedEventHandler<winrt::Microsoft::UI::Xaml::Controls::Layout,winrt::Windows::Foundation::IInspectable>,<lambda_5d92c9e3076376e1f0ac0972ab590101> >::Invoke+0x3b  (7ffc5425b9fb) [C:\a\_work\1\s\BuildOutput\Intermediates\x64\Microsoft.UI.Xaml\obj\Generated Files\winrt\Windows.Foundation.h @ 883]
    Microsoft_UI_Xaml!winrt::Windows::Foundation::Collections::VectorChangedEventHandler<winrt::Windows::Foundation::IInspectable>::operator  (7ffc54263f4a) [C:\a\_work\1\s\BuildOutput\Intermediates\x64\Microsoft.UI.Xaml\obj\Generated Files\winrt\Windows.Foundation.Collections.h @ 690]
    Microsoft_UI_Xaml!event_base<event_source<winrt::Windows::Foundation::Collections::VectorChangedEventHandler<winrt::Windows::Foundation::IInspectable> >,winrt::Windows::Foundation::Collections::VectorChangedEventHandler<winrt::Windows::Foundation::IInspectable>,tracker_ref<winrt::Windows::Foundation::Collections::VectorChangedEventHandler<winrt::Windows::Foundation::IInspectable>,0,IUnknown *> >::operator  (7ffc54263eef) [C:\a\_work\1\s\dev\inc\event.h @ 77]
    Microsoft_UI_Xaml!ObservableVectorInnerImpl<VectorOptions<winrt::Windows::Foundation::IInspectable,1,0,1,0>,TStorageWrapperImpl<winrt::Windows::Foundation::IInspectable,0> >::RaiseChildrenChanged+0x118  (7ffc54263df8) [C:\a\_work\1\s\dev\Collections\Vector.h @ 341]
    Microsoft_UI_Xaml!winrt::impl::produce<VectorBase<winrt::Windows::Foundation::IInspectable,1,0,1,0,VectorOptions<winrt::Windows::Foundation::IInspectable,1,0,1,0> >,winrt::Windows::Foundation::Collections::IVector<winrt::Windows::Foundation::IInspectable> >::ReplaceAll+0x4f  (7ffc542f16df) [C:\a\_work\1\s\BuildOutput\Intermediates\x64\Microsoft.UI.Xaml\obj\Generated Files\winrt\Windows.Foundation.Collections.h @ 624]
    Microsoft_Terminal_Settings_Editor!winrt::Microsoft::Terminal::Settings::Editor::implementation::MainPage::UpdateSettings+0x1db

That looks more right. MainPage::UpdateSettings -> replaces all the elements of the nav view -> NavigationView::OnMenuItemsSourceCollectionChanged

@zadjii-msft commented on GitHub (Aug 5, 2022): Hmm. Coroutines are weird, but this looks like the error it's trying to report is `"The operation attempted to access data outside the valid range.."`. Interesting bit of the stack is: ``` 2e (Inline Function) --------`-------- rpcrt4!Ndr64pSendReceive+0x2f [minio\rpc\ndr64\cltcall.cxx @ 184] 2f 000000aa`24d9dab0 00007ffc`e6abc538 rpcrt4!NdrpClientCall3+0x3a4 [minio\rpc\ndr64\cltcall.cxx @ 328] 30 000000aa`24d9de20 00007ffc`e6bcf642 combase!ObjectStublessClient+0x138 [onecore\com\combase\ndr\ndrole\amd64\stblsclt.cxx @ 369] 31 000000aa`24d9e1b0 00007ffc`613f652d combase!ObjectStubless+0x42 [onecore\com\combase\ndr\ndrole\amd64\stubless.asm @ 176] 32 000000aa`24d9e200 00007ffc`613a8d24 TerminalApp!winrt::impl::consume_Windows_ApplicationModel_IStartupTask<winrt::Windows::ApplicationModel::IStartupTask>::State+0x2d [C:\a\_work\1\s\src\cascadia\TerminalApp\Generated Files\winrt\Windows.ApplicationModel.h @ 1077] 33 000000aa`24d9e240 00007ffc`613a4d71 TerminalApp!winrt::TerminalApp::implementation::AppLogic::_ApplyStartupTaskStateChange$_ResumeCoro$1+0x1c4 [C:\a\_work\1\s\src\cascadia\TerminalApp\AppLogic.cpp @ 1004] 34 (Inline Function) --------`-------- TerminalApp!std::experimental::coroutine_handle<void>::resume+0x9 ``` Kinda looks like ```c++ const auto task = co_await StartupTask::GetAsync(StartupTaskName); switch (task.State()) { ``` Like, `task` was null? <hr> Naw, this is in xaml. There's a stowed xaml exception: ``` Windows_UI_Xaml!CUIElement::Measure+0x4bd (7ffccced5dfd) [onecoreuap\windows\dxaml\xcp\core\core\elements\uielement.cpp @ 4063] Windows_UI_Xaml!CUIElement::Measure+0x4bd (7ffccced5dfd) [onecoreuap\windows\dxaml\xcp\core\core\elements\uielement.cpp @ 4063] Windows_UI_Xaml!CUIElement::Measure+0x4bd (7ffccced5dfd) [onecoreuap\windows\dxaml\xcp\core\core\elements\uielement.cpp @ 4063] Windows_UI_Xaml!CLayoutManager::UpdateLayout+0x107 (7ffcccecc987) [onecoreuap\windows\dxaml\xcp\core\layout\layoutmanager.cpp @ 279] Windows_UI_Xaml!CUIElement::UpdateLayout+0x3d (7ffccce3d309) [onecoreuap\windows\dxaml\xcp\core\core\elements\uielement.cpp @ 3761] Windows_UI_Xaml!DirectUI::UIElementGenerated::UpdateLayout+0x47 (7ffccce3cac7) [onecoreuap\windows\dxaml\xcp\dxaml\lib\winrtgeneratedclasses\uielement.g.cpp @ 4804] Microsoft_UI_Xaml!winrt::impl::consume_Windows_UI_Xaml_IUIElement<winrt::Microsoft::UI::Xaml::Controls::ItemsRepeater>::UpdateLayout+0x53 (7ffc5427ebd3) [C:\a\_work\1\s\BuildOutput\Intermediates\x64\Microsoft.UI.Xaml\obj\Generated Files\winrt\Windows.UI.Xaml.h @ 3021] Microsoft_UI_Xaml!NavigationView::OnMenuItemsSourceCollectionChanged+0x24 (7ffc543041b4) [C:\a\_work\1\s\dev\NavigationView\NavigationView.cpp @ 3251] Microsoft_UI_Xaml!winrt::impl::delegate<winrt::Windows::UI::Xaml::SizeChangedEventHandler,<lambda_778e218533f9d63cc7aafcdd2f5bac52> >::Invoke+0x3b (7ffc542cd5db) [C:\a\_work\1\s\BuildOutput\Intermediates\x64\Microsoft.UI.Xaml\obj\Generated Files\winrt\Windows.Foundation.h @ 894] Microsoft_UI_Xaml!event_base<event_source<winrt::Windows::UI::Xaml::Interop::NotifyCollectionChangedEventHandler>,winrt::Windows::UI::Xaml::Interop::NotifyCollectionChangedEventHandler,tracker_ref<winrt::Windows::UI::Xaml::Interop::NotifyCollectionChangedEventHandler,0,IUnknown *> >::operator (7ffc5421fd87) [C:\a\_work\1\s\dev\inc\event.h @ 77] Microsoft_UI_Xaml!InspectingDataSource::OnVectorChanged+0xe4 (7ffc541d4294) [C:\a\_work\1\s\dev\Repeater\InspectingDataSource.cpp @ 260] Microsoft_UI_Xaml!winrt::impl::delegate<winrt::Windows::Foundation::TypedEventHandler<winrt::Microsoft::UI::Xaml::Controls::Layout,winrt::Windows::Foundation::IInspectable>,<lambda_5d92c9e3076376e1f0ac0972ab590101> >::Invoke+0x3b (7ffc5425b9fb) [C:\a\_work\1\s\BuildOutput\Intermediates\x64\Microsoft.UI.Xaml\obj\Generated Files\winrt\Windows.Foundation.h @ 883] Microsoft_UI_Xaml!winrt::Windows::Foundation::Collections::VectorChangedEventHandler<winrt::Windows::Foundation::IInspectable>::operator (7ffc54263f4a) [C:\a\_work\1\s\BuildOutput\Intermediates\x64\Microsoft.UI.Xaml\obj\Generated Files\winrt\Windows.Foundation.Collections.h @ 690] Microsoft_UI_Xaml!event_base<event_source<winrt::Windows::Foundation::Collections::VectorChangedEventHandler<winrt::Windows::Foundation::IInspectable> >,winrt::Windows::Foundation::Collections::VectorChangedEventHandler<winrt::Windows::Foundation::IInspectable>,tracker_ref<winrt::Windows::Foundation::Collections::VectorChangedEventHandler<winrt::Windows::Foundation::IInspectable>,0,IUnknown *> >::operator (7ffc54263eef) [C:\a\_work\1\s\dev\inc\event.h @ 77] Microsoft_UI_Xaml!ObservableVectorInnerImpl<VectorOptions<winrt::Windows::Foundation::IInspectable,1,0,1,0>,TStorageWrapperImpl<winrt::Windows::Foundation::IInspectable,0> >::RaiseChildrenChanged+0x118 (7ffc54263df8) [C:\a\_work\1\s\dev\Collections\Vector.h @ 341] Microsoft_UI_Xaml!winrt::impl::produce<VectorBase<winrt::Windows::Foundation::IInspectable,1,0,1,0,VectorOptions<winrt::Windows::Foundation::IInspectable,1,0,1,0> >,winrt::Windows::Foundation::Collections::IVector<winrt::Windows::Foundation::IInspectable> >::ReplaceAll+0x4f (7ffc542f16df) [C:\a\_work\1\s\BuildOutput\Intermediates\x64\Microsoft.UI.Xaml\obj\Generated Files\winrt\Windows.Foundation.Collections.h @ 624] Microsoft_Terminal_Settings_Editor!winrt::Microsoft::Terminal::Settings::Editor::implementation::MainPage::UpdateSettings+0x1db ``` That looks more right. `MainPage::UpdateSettings` -> replaces all the elements of the nav view -> `NavigationView::OnMenuItemsSourceCollectionChanged`

claunia commented

@ashemedai commented on GitHub (Aug 6, 2022):

If I need to look at anything from VS 2022's debugger view for you, let me know. Just be a bit more explicit, since I hardly ever use that.

@ashemedai commented on GitHub (Aug 6, 2022): If I need to look at anything from VS 2022's debugger view for you, let me know. Just be a bit more explicit, since I hardly ever use that.

claunia commented

@zadjii-msft commented on GitHub (Aug 10, 2022):

Nah, I think we can repro this pretty readily. This is probably just more fallout from doing weird stuff to our nav view. Doing this a few times seemed like I would get a friggen wild set of items in the nave view, until eventually the app would crash.

@zadjii-msft commented on GitHub (Aug 10, 2022): Nah, I think we can repro this pretty readily. This is probably just more fallout from doing weird stuff to our nav view. Doing this a few times seemed like I would get a friggen wild set of items in the nave view, until eventually the app would crash.

claunia commented

@ashemedai commented on GitHub (Aug 31, 2022):

@zadjii-msft Did you need a (mini)dump for this?

@ashemedai commented on GitHub (Aug 31, 2022): @zadjii-msft Did you need a (mini)dump for this?

claunia commented

@zadjii-msft commented on GitHub (Aug 31, 2022):

Nah, this one's got a consistent repro. Just need someone to take the time to look into it at this point.

@zadjii-msft commented on GitHub (Aug 31, 2022): Nah, this one's got a consistent repro. Just need someone to take the time to look into it at this point.

claunia commented

@zadjii-msft commented on GitHub (Jan 4, 2023):

EDIT

Yes that absolutely works. It's horrifying but it works.

@zadjii-msft commented on GitHub (Jan 4, 2023): @chingucoding Okay, I'm way out on a limb here. But I was looking at https://github.com/microsoft/microsoft-ui-xaml/pull/3138, which was the fix for https://github.com/microsoft/microsoft-ui-xaml/issues/2818, and thought you might have an idea here. TL;DR: we're calling `IVector<>::ReplaceAll()` on our `NavigationView`'s `MenuItems` to replace the list when the settings reloads (demonstrated above by just deleting your existing settings, forcing us to generate a fresh set). However, when the list decreases in length[1], it seems like the NavView just _crashes_. You think if I somehow converted our `std::vector<MUX::Controls::NavigationViewItem>` into something that could be used as a `MenuItemSource`, that I could swap out the entries that way? [1]: it's not _totally_ clear what triggers the crash, but that seems close enough to the truth. ## EDIT Yes that absolutely works. It's horrifying but it works.

claunia commented

@marcelwgn commented on GitHub (Jan 5, 2023):

That's a really interesting one @zadjii-msft and I'm surprised that works. Out of curiosity, is that the fix you want to use? Reading the title of the issue, could this maybe be caused because the new collection is empty? None the less, this is a very interesting bug, thanks for mentioning me.

@marcelwgn commented on GitHub (Jan 5, 2023): That's a really interesting one @zadjii-msft and I'm surprised that works. Out of curiosity, is that the fix you want to use? Reading the title of the issue, could this maybe be caused because the new collection is empty? None the less, this is a very interesting bug, thanks for mentioning me.

claunia commented

@zadjii-msft commented on GitHub (Jan 5, 2023):

Is that the fix I want to use? Probably not. Am I willing though? Absolutely.

maybe be caused because the new collection is empty

Through a series of other things that transpire, the new collection actually isn't empty. When we see that the new settings file is empty, we immediately blat in the default settings, then parse that, so the new collection should have Command Prompt, PowerShell, and possibly a couple more profiles at the least.

Seems to me like anytime you remove an element from the nav view's MenuItems, we hit that exception, even if there's plenty of other elements left. Wild, right?

@zadjii-msft commented on GitHub (Jan 5, 2023): Is that the fix I _want_ to use? Probably not. Am I willing though? Absolutely. > maybe be caused because the new collection is empty Through a series of other things that transpire, the new collection actually isn't empty. When we see that the new settings file is empty, we immediately blat in the default settings, then parse that, so the new collection should have Command Prompt, PowerShell, and possibly a couple more profiles at the least. Seems to me like anytime you remove an element from the nav view's `MenuItems`, we hit that exception, even if there's plenty of other elements left. Wild, right?

claunia commented

@marcelwgn commented on GitHub (Jan 5, 2023):

Ah I see, so the collection is not empty. It's really strange that this issue happens to you, I was not able to reproduce this (though I was trying this in C# which does not have a replace). Do I understand this correctly that Clear/Remove/... also doesn't work?

@marcelwgn commented on GitHub (Jan 5, 2023): Ah I see, so the collection is not empty. It's really strange that this issue happens to you, I was not able to reproduce this (though I was trying this in C# which does not have a replace). Do I understand this correctly that Clear/Remove/... also doesn't work?

claunia commented

@marcelwgn commented on GitHub (Jan 5, 2023):

~~An alternative I could think of would be to not call ReplaceAll on the Menuitems but rather set the MenuItems property on the NavigationView. Maybe that doesn't crash?~~ That property seems to be readonly so I guess that doesn't work either.

@marcelwgn commented on GitHub (Jan 5, 2023): ~An alternative I could think of would be to not call `ReplaceAll` on the Menuitems but rather set the MenuItems property on the NavigationView. Maybe that doesn't crash?~ That property seems to be readonly so I guess that doesn't work either.

claunia commented