terminal icon indicating copy to clipboard operation
terminal copied to clipboard

Published 20 hours ago • verified publisher icon microsoft

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

Open ashemedai opened this issue 2 years ago • 15 comments

Windows Terminal version

1.14.1962.0

Windows build number

10.0.19043.0

Other Software

N/A

Steps to reproduce

  1. Open Terminal
  2. Open Settings
  3. Open JSON file
  4. In external editor, CTRL+A to select the contents of the settings.json and press delete
  5. Save

Expected Behavior

No crash.

Actual Behavior

Windows Terminal repopulates the settings.json file with defaults and then crashes with:

Unhandled exception at 0x00007FFCE568FB62 (KernelBase.dll) in WindowsTerminal.exe: 0xC000027B: An application-internal exception has occurred (parameters: 0x000002CDCB9A0970, 0x000000000000000A).

Which is being triggered from TerminalApp.dll!winrt::TerminalApp::implementation::AppLogic::_ApplyStartupTaskStateChange$_ResumeCoro$1() Line 1004.

Edit: Feedback Hub URL: https://aka.ms/AAhm2q3

ashemedai avatar Aug 04 '22 17:08 ashemedai

Possibly related to #12333.

ashemedai avatar Aug 04 '22 17:08 ashemedai

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 avatar Aug 05 '22 18:08 zadjii-msft

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 avatar Aug 05 '22 19:08 ashemedai

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 avatar Aug 05 '22 20:08 zadjii-msft

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 avatar Aug 06 '22 08:08 ashemedai

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 avatar Aug 10 '22 21:08 zadjii-msft

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

ashemedai avatar Aug 31 '22 12:08 ashemedai

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

zadjii-msft avatar Aug 31 '22 15:08 zadjii-msft

see also:

  • #9273
  • #10390

This is the SAME THING.

https://github.com/microsoft/terminal/blob/2d66dc44f5abef975bed86af12cb5facb6729a1a/src/cascadia/TerminalSettingsEditor/MainPage.cpp#L152-L157

That ReplaceAll call, causes a XAML exception higher in the stack.

We can't

  • Clear
  • RemoveAtEnd in a loop
  • Just RemoveAtEnd once: nope
  • Just RemoveAt(0) once: that crashes too.

Hmm.

zadjii-msft avatar Jan 04 '23 20:01 zadjii-msft

@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.

zadjii-msft avatar Jan 04 '23 20:01 zadjii-msft

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 avatar Jan 05 '23 17:01 marcelwgn

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 avatar Jan 05 '23 17:01 zadjii-msft

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 avatar Jan 05 '23 18:01 marcelwgn

~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 avatar Jan 05 '23 18:01 marcelwgn

Do I understand this correctly that Clear/Remove/... also doesn't work?

Sorry I missed this - yea no variation on Remove seemed to work. I tried Clear, RemoveAtEnd in a loop, just RemoveAtEnding once, just RemoveAt(0) once - all crashes. Wild, I know.

zadjii-msft avatar Jan 17 '23 23:01 zadjii-msft