Windows Terminal throws an exception on exit #19880

Closed
opened 2026-01-31 06:56:11 +00:00 by claunia · 14 comments
Owner

Originally created by @j4james on GitHub (May 16, 2023).

Originally assigned to: @zadjii-msft on GitHub.

Windows Terminal version

Commit ba39db52d7

Windows build number

10.0.19045.2728

Other Software

No response

Steps to reproduce

  1. Be on Windows 10 (I suspect this is necessary).
  2. Check out a recent commmit (ba39db52d7 or later).
  3. Build and run Windows Terminal from within Visual Studio.
  4. Exit the app.

Expected Behavior

It should shutdown gracefully.

Actual Behavior

It crashes with an exception:

Exception thrown at 0x00007FFCFE18D08E (Windows.UI.Xaml.dll) in WindowsTerminal.exe: 0xC0000005: Access violation reading location 0x0000000000000130.

Stack trace:

 	[Inline Frame] Windows.UI.Xaml.dll!DirectUI::DXamlCore::GetWindow() Line 278	C++
 	Windows.UI.Xaml.dll!DirectUI::ContentDialog::DetachEventHandlersForOpenDialog() Line 2507	C++
 	Windows.UI.Xaml.dll!DirectUI::ContentDialog::~ContentDialog() Line 74	C++
 	Windows.UI.Xaml.dll!ctl::ComObject<DirectUI::ContentDialog>::`scalar deleting destructor'(unsigned int)	C++
 	Windows.UI.Xaml.dll!ctl::ComBase::ReleaseImpl() Line 307	C++
>	TerminalApp.dll!winrt::TerminalApp::implementation::TerminalWindow::~TerminalWindow() Line 68	C++
 	[External Code]	
 	TerminalApp.dll!winrt::implements<winrt::TerminalApp::implementation::TerminalWindow,winrt::TerminalApp::TerminalWindow,winrt::TerminalApp::IDirectKeyListener,winrt::TerminalApp::IDialogPresenter,winrt::Windows::UI::Xaml::Data::INotifyPropertyChanged,IInitializeWithWindow>::Release() Line 8118	C++
 	[External Code]	
 	[Inline Frame] WindowsTerminal.exe!std::_Func_class<float,bool,float>::_Tidy() Line 952	C++
 	[Inline Frame] WindowsTerminal.exe!std::_Func_class<float,bool,float>::{dtor}() Line 878	C++
 	WindowsTerminal.exe!IslandWindow::~IslandWindow() Line 41	C++
 	[External Code]	

This looks very similar to #11980 and #13955, so I wouldn't be surprised if this is the MSFT:26130824 bug that is supposed to have been fixed in Windows 11. However, in my case there are no dialogs popping up, and there is nothing wrong with the settings (I reset my settings.json to the defaults).

Most importantly, though, I've only had this issue since PR #15338 was merged, and now I get it every time I run the app from within VS. So I don't mind if you also end up closing this as Resolution-External, but I thought it worth raising in case there's a more serious regression here.

Originally created by @j4james on GitHub (May 16, 2023). Originally assigned to: @zadjii-msft on GitHub. ### Windows Terminal version Commit ba39db52d71024584267746f777156a94caceeb0 ### Windows build number 10.0.19045.2728 ### Other Software _No response_ ### Steps to reproduce 1. Be on Windows 10 (I suspect this is necessary). 2. Check out a recent commmit (ba39db52d71024584267746f777156a94caceeb0 or later). 3. Build and run Windows Terminal from within Visual Studio. 4. Exit the app. ### Expected Behavior It should shutdown gracefully. ### Actual Behavior It crashes with an exception: > Exception thrown at 0x00007FFCFE18D08E (Windows.UI.Xaml.dll) in WindowsTerminal.exe: 0xC0000005: Access violation reading location 0x0000000000000130. Stack trace: ``` [Inline Frame] Windows.UI.Xaml.dll!DirectUI::DXamlCore::GetWindow() Line 278 C++ Windows.UI.Xaml.dll!DirectUI::ContentDialog::DetachEventHandlersForOpenDialog() Line 2507 C++ Windows.UI.Xaml.dll!DirectUI::ContentDialog::~ContentDialog() Line 74 C++ Windows.UI.Xaml.dll!ctl::ComObject<DirectUI::ContentDialog>::`scalar deleting destructor'(unsigned int) C++ Windows.UI.Xaml.dll!ctl::ComBase::ReleaseImpl() Line 307 C++ > TerminalApp.dll!winrt::TerminalApp::implementation::TerminalWindow::~TerminalWindow() Line 68 C++ [External Code] TerminalApp.dll!winrt::implements<winrt::TerminalApp::implementation::TerminalWindow,winrt::TerminalApp::TerminalWindow,winrt::TerminalApp::IDirectKeyListener,winrt::TerminalApp::IDialogPresenter,winrt::Windows::UI::Xaml::Data::INotifyPropertyChanged,IInitializeWithWindow>::Release() Line 8118 C++ [External Code] [Inline Frame] WindowsTerminal.exe!std::_Func_class<float,bool,float>::_Tidy() Line 952 C++ [Inline Frame] WindowsTerminal.exe!std::_Func_class<float,bool,float>::{dtor}() Line 878 C++ WindowsTerminal.exe!IslandWindow::~IslandWindow() Line 41 C++ [External Code] ``` This looks very similar to #11980 and #13955, so I wouldn't be surprised if this is the MSFT:26130824 bug that is supposed to have been fixed in Windows 11. However, in my case there are no dialogs popping up, and there is nothing wrong with the settings (I reset my settings.json to the defaults). Most importantly, though, I've only had this issue since PR #15338 was merged, and now I get it every time I run the app from within VS. So I don't mind if you also end up closing this as Resolution-External, but I thought it worth raising in case there's a more serious regression here.
Author
Owner

@lhecker commented on GitHub (May 16, 2023):

Oh, I wonder if we need to release all the WinUI members from IslandWindow / NonClientIslandWindow‎ when Close() is being called. If this is something you'd like to try, I think that could fix it. Otherwise I'll set up a Windows 10 VM sometime soon and try to see if I can debug this.

@lhecker commented on GitHub (May 16, 2023): Oh, I wonder if we need to release all the WinUI members from `IslandWindow` / `NonClientIslandWindow‎` when `Close()` is being called. If this is something you'd like to try, I think that could fix it. Otherwise I'll set up a Windows 10 VM sometime soon and try to see if I can debug this.
Author
Owner

@lhecker commented on GitHub (May 16, 2023):

Also, I think I just realized that we might want to also call

SetWindowLongPtr(GetHandle(), GWLP_USERDATA, 0);

in IslandWindow::Close(). This would ensure that we don't end up accessing any of the nulled out members. 🤔 Something like this:

void IslandWindow::Close()
{
    if (_source)
    {
        SetWindowLongPtr(GetHandle(), GWLP_USERDATA, 0);
        _rootGrid = nullptr;
        _source.Close();
        _source = nullptr;
    }
}
@lhecker commented on GitHub (May 16, 2023): Also, I think I just realized that we might want to also call ```cpp SetWindowLongPtr(GetHandle(), GWLP_USERDATA, 0); ``` in `IslandWindow::Close()`. This would ensure that we don't end up accessing any of the nulled out members. 🤔 Something like this: ```cpp void IslandWindow::Close() { if (_source) { SetWindowLongPtr(GetHandle(), GWLP_USERDATA, 0); _rootGrid = nullptr; _source.Close(); _source = nullptr; } } ```
Author
Owner

@j4james commented on GitHub (May 16, 2023):

I've update IslandWindow as above, and NonClientIslandWindow like this:

    SetWindowLongPtr(_dragBarWindow.get(), GWLP_USERDATA, 0);
    IslandWindow::Close();
    _dragBar = nullptr;
    _clientContent = nullptr;

But it's still crashing.

@j4james commented on GitHub (May 16, 2023): I've update `IslandWindow` as above, and `NonClientIslandWindow` like this: ```cpp SetWindowLongPtr(_dragBarWindow.get(), GWLP_USERDATA, 0); IslandWindow::Close(); _dragBar = nullptr; _clientContent = nullptr; ``` But it's still crashing.
Author
Owner

@lhecker commented on GitHub (May 16, 2023):

Ah too bad... It probably won't fix it then. Although I think you'd have to put the 2 = nullptr assignments before the call to IslandWindow::Close();. IslandWindow::Close(); is what ends up calling Close() on the XAML island, at which point WinUI starts behaving "weird".

@lhecker commented on GitHub (May 16, 2023): Ah too bad... It probably won't fix it then. Although I think you'd have to put the 2 `= nullptr` assignments before the call to `IslandWindow::Close();`. `IslandWindow::Close();` is what ends up calling `Close()` on the XAML island, at which point WinUI starts behaving "weird".
Author
Owner

@lhecker commented on GitHub (May 16, 2023):

OOOOH... It's crashing in TerminalWindow. Does this mean we have to null out the _windowLogic early in AppHost::Close then? I'm just thinking out loud though - please don't feel obligated to spend time fixing our bugs. That's already our job after all. 😅

@lhecker commented on GitHub (May 16, 2023): OOOOH... It's crashing in `TerminalWindow`. Does this mean we have to null out the `_windowLogic` early in `AppHost::Close` then? I'm just thinking out loud though - please don't feel obligated to spend time fixing our bugs. That's already our job after all. 😅
Author
Owner

@j4james commented on GitHub (May 16, 2023):

OK, I've tried with the two nullptr assignments before the IslandWindow::Close() and with _windowLogic nulled out as the first thing in AppHost::Close. Neither of those changes seem to make any difference.

If it helps, looking at the disassembly for the TerminalWindow destructor, the crash is happening inside the unconditional_release_ref call here:

00007FFC6A1729A9  lea         rcx,[rbx+70h]  
00007FFC6A1729AD  call        TerminalApp::AppCommandlineArgs::~AppCommandlineArgs (07FFC6A16D260h)  
00007FFC6A1729B2  cmp         qword ptr [rbx+58h],0  
00007FFC6A1729B7  lea         rcx,[rbx+58h]  
00007FFC6A1729BB  je          winrt::TerminalApp::implementation::TerminalWindow::~TerminalWindow+0E2h (07FFC6A1729C2h)  
00007FFC6A1729BD >call        winrt::com_ptr<winrt::impl::IErrorInfo>::unconditional_release_ref (07FFC6A159460h)  
00007FFC6A1729C2  cmp         qword ptr [rbx+50h],0  
00007FFC6A1729C7  lea         rcx,[rbx+50h]  
00007FFC6A1729CB  je          winrt::TerminalApp::implementation::TerminalWindow::~TerminalWindow+0F2h (07FFC6A1729D2h)  
00007FFC6A1729CD  call        winrt::com_ptr<winrt::TerminalApp::implementation::AboutDialog>::unconditional_release_ref (07FFC6A162EB0h)  

So that looks to me like _appArgs has been destructed, and the problem is occurring in whatever comes next. Would that be the _dialog field?

You're welcome to suggest other things for me to try, as long as you don't mind the slow turnaround time.

@j4james commented on GitHub (May 16, 2023): OK, I've tried with the two nullptr assignments before the `IslandWindow::Close()` and with `_windowLogic` nulled out as the first thing in `AppHost::Close`. Neither of those changes seem to make any difference. If it helps, looking at the disassembly for the `TerminalWindow` destructor, the crash is happening inside the `unconditional_release_ref` call here: ``` 00007FFC6A1729A9 lea rcx,[rbx+70h] 00007FFC6A1729AD call TerminalApp::AppCommandlineArgs::~AppCommandlineArgs (07FFC6A16D260h) 00007FFC6A1729B2 cmp qword ptr [rbx+58h],0 00007FFC6A1729B7 lea rcx,[rbx+58h] 00007FFC6A1729BB je winrt::TerminalApp::implementation::TerminalWindow::~TerminalWindow+0E2h (07FFC6A1729C2h) 00007FFC6A1729BD >call winrt::com_ptr<winrt::impl::IErrorInfo>::unconditional_release_ref (07FFC6A159460h) 00007FFC6A1729C2 cmp qword ptr [rbx+50h],0 00007FFC6A1729C7 lea rcx,[rbx+50h] 00007FFC6A1729CB je winrt::TerminalApp::implementation::TerminalWindow::~TerminalWindow+0F2h (07FFC6A1729D2h) 00007FFC6A1729CD call winrt::com_ptr<winrt::TerminalApp::implementation::AboutDialog>::unconditional_release_ref (07FFC6A162EB0h) ``` So that looks to me like `_appArgs` has been destructed, and the problem is occurring in whatever comes next. Would that be the `_dialog` field? You're welcome to suggest other things for me to try, as long as you don't mind the slow turnaround time.
Author
Owner

@zadjii-msft commented on GitHub (May 17, 2023):

From the internal bug:

It looks like the crashing apps are releasing a reference to ContentDialog after the XAML core has been released while the thread/app is shutting down, which means DXamlCore::GetCurrent() returns null and leads to a crash

that might be a clue.

I'm inclined to not call this 1.18 blocking?

@zadjii-msft commented on GitHub (May 17, 2023): From the internal bug: > It looks like the crashing apps are releasing a reference to ContentDialog after the XAML core has been released while the thread/app is shutting down, which means DXamlCore::GetCurrent() returns null and leads to a crash that might be a clue. I'm inclined to _not_ call this 1.18 blocking?
Author
Owner

@j4james commented on GitHub (May 18, 2023):

It looks like the crashing apps are releasing a reference to ContentDialog after the XAML core has been released while the thread/app is shutting down, which means DXamlCore::GetCurrent() returns null and leads to a crash

that might be a clue.

That inspired me to try calling _windowLogic.DismissDialog() from within the AppHost::Close method, so the dialog is released earlier in the shutdown, and that seems to have fixed the issue for me. Also still works after removing all the other patches we tried. I don't know if that's the right solution, but it's a starting point at least.

I'm inclined to not call this 1.18 blocking?

Yeah. As far as I can see, it's not noticeable outside the debugger, so it doesn't seem like a big deal.

@j4james commented on GitHub (May 18, 2023): > > It looks like the crashing apps are releasing a reference to ContentDialog after the XAML core has been released while the thread/app is shutting down, which means DXamlCore::GetCurrent() returns null and leads to a crash > > that might be a clue. That inspired me to try calling `_windowLogic.DismissDialog()` from within the `AppHost::Close` method, so the dialog is released earlier in the shutdown, and that seems to have fixed the issue for me. Also still works after removing all the other patches we tried. I don't know if that's the _right_ solution, but it's a starting point at least. > I'm inclined to not call this 1.18 blocking? Yeah. As far as I can see, it's not noticeable outside the debugger, so it doesn't seem like a big deal.
Author
Owner

@j4james commented on GitHub (May 18, 2023):

Just to be absolutely clear, my working AppHost::Close method now looks like this:

void AppHost::Close()
{
    // After calling _window->Close() we should avoid creating more WinUI related actions.
    // I suspect WinUI wouldn't like that very much. As such unregister all event handlers first.
    _revokers = {};
    _showHideWindowThrottler.reset();
    _window->Close();
    if (_windowLogic)
    {
        _windowLogic.DismissDialog();
        _windowLogic = nullptr;
    }
}
@j4james commented on GitHub (May 18, 2023): Just to be absolutely clear, my working `AppHost::Close` method now looks like this: ```cpp void AppHost::Close() { // After calling _window->Close() we should avoid creating more WinUI related actions. // I suspect WinUI wouldn't like that very much. As such unregister all event handlers first. _revokers = {}; _showHideWindowThrottler.reset(); _window->Close(); if (_windowLogic) { _windowLogic.DismissDialog(); _windowLogic = nullptr; } } ```
Author
Owner

@zadjii-msft commented on GitHub (May 18, 2023):

What the everliving.... I think I'm seeing totally other crashes? I'm gonna leave a note here but probably instantly mark off-topic unless I can establish this is the same root cause

These are from me deploying to a Win10 19041 VM.

Crash the first

Closed a second window, then hovered over the first. At some point got:

Exception thrown at 0x00007FFE1C984F99 (KernelBase.dll) in WindowsTerminal.exe: WinRT originate error - 0x80004005 : 'The property 'TabStripHeader' was not found in type 'Microsoft.UI.Xaml.Controls.TabView'.'.
Exception thrown at 0x00007FFE1C984F99 (KernelBase.dll) in WindowsTerminal.exe: WinRT originate error - 0x802B000A : 'The property 'TabStripHeader' was not found in type 'Microsoft.UI.Xaml.Controls.TabView'. [Line: 2577 Position: 79]'.

Never grabbed the stack. It was up in XAML land, and it was in _messagePump, seemingly in a normal state? I was able to continue past this.

When I exited that window (the last), then the debugger never exited, and we stayed in the message loop forever. w e i r d.

Crash the second

Closed a second window, then hovered over the first. At some point got:

>	Windows.UI.Xaml.Controls.dll!winrt::throw_hresult(const winrt::hresult result) Line 4524	C++
 	[Inline Frame] Windows.UI.Xaml.Controls.dll!winrt::check_hresult(winrt::hresult) Line 4612	C++
 	Windows.UI.Xaml.Controls.dll!winrt::impl::consume_Windows_UI_Xaml_IDependencyObject<AcrylicBrush>::GetValue(const winrt::Windows::UI::Xaml::DependencyProperty & dp) Line 1045	C++
 	[Inline Frame] Windows.UI.Xaml.Controls.dll!AcrylicBrushProperties::AlwaysUseFallback() Line 150	C++
 	Windows.UI.Xaml.Controls.dll!AcrylicBrush::UpdateAcrylicBrush() Line 996	C++
    ...

This one's a hard failfast that can't be continued. Window also seems to be in a normal state? In a normal _messagePump?

This happens every time I hover the split button after closing another window on Windows 10, but not on Windows 11. w a t


what is going on here.

@zadjii-msft commented on GitHub (May 18, 2023): What the everliving.... I think I'm seeing totally other crashes? I'm gonna leave a note here but probably instantly mark off-topic unless I can establish this is the same root cause These are from me deploying to a Win10 19041 VM. ### Crash the first Closed a second window, then hovered over the first. At some point got: ``` Exception thrown at 0x00007FFE1C984F99 (KernelBase.dll) in WindowsTerminal.exe: WinRT originate error - 0x80004005 : 'The property 'TabStripHeader' was not found in type 'Microsoft.UI.Xaml.Controls.TabView'.'. Exception thrown at 0x00007FFE1C984F99 (KernelBase.dll) in WindowsTerminal.exe: WinRT originate error - 0x802B000A : 'The property 'TabStripHeader' was not found in type 'Microsoft.UI.Xaml.Controls.TabView'. [Line: 2577 Position: 79]'. ``` Never grabbed the stack. It was up in XAML land, and it was in `_messagePump`, seemingly in a normal state? I was able to continue past this. When I exited that window (the last), then the debugger never exited, and we stayed in the message loop forever. _w e i r d_. ### Crash the second Closed a second window, then hovered over the first. At some point got: ``` > Windows.UI.Xaml.Controls.dll!winrt::throw_hresult(const winrt::hresult result) Line 4524 C++ [Inline Frame] Windows.UI.Xaml.Controls.dll!winrt::check_hresult(winrt::hresult) Line 4612 C++ Windows.UI.Xaml.Controls.dll!winrt::impl::consume_Windows_UI_Xaml_IDependencyObject<AcrylicBrush>::GetValue(const winrt::Windows::UI::Xaml::DependencyProperty & dp) Line 1045 C++ [Inline Frame] Windows.UI.Xaml.Controls.dll!AcrylicBrushProperties::AlwaysUseFallback() Line 150 C++ Windows.UI.Xaml.Controls.dll!AcrylicBrush::UpdateAcrylicBrush() Line 996 C++ ... ``` This one's a hard failfast that can't be continued. Window also seems to be in a normal state? In a normal `_messagePump`? #### This happens every time I hover the split button after closing another window on Windows 10, but not on Windows 11. w a t ---- what is going on here.
Author
Owner

@j4james commented on GitHub (May 18, 2023):

This happens every time I hover the split button after closing another window on Windows 10

I can reproduce this as well, but only outside the debugger, and only after applying the AppHost patch from above. Without the patch, both instances die as soon as I close one of them (although sometimes they freeze for a couple of seconds before they die, but they're as good as dead at that point).

Edit: I don't typically use multiple windows myself, but for people that do, this does seems like a big deal, so maybe worth reconsidering as a 1.18 blocker.

@j4james commented on GitHub (May 18, 2023): > This happens every time I hover the split button after closing another window on Windows 10 I can reproduce this as well, but only outside the debugger, and only after applying the `AppHost` patch from above. Without the patch, both instances die as soon as I close one of them (although sometimes they freeze for a couple of seconds before they die, but they're as good as dead at that point). Edit: I don't typically use multiple windows myself, but for people that do, this does seems like a big deal, so maybe worth reconsidering as a 1.18 blocker.
Author
Owner

@zadjii-msft commented on GitHub (May 18, 2023):

Oh it's definitely been promoted to a 1.18 block, don't worry about that. I'm inclined to say that your proposal here works and is fairly minimal, so safe to bring in (over some more convoluted lifetime management changes).

I'm trying to dig in to the source of the button one. I've got this horrible theory that it's something that was fixed in Windows 11. I need to consult an expert here. You're on 10.0.19045.2728, so that at least would be quite a bit more up to date than my VM. I'm gonna fork that to another thread.

@zadjii-msft commented on GitHub (May 18, 2023): Oh it's definitely been promoted to a 1.18 block, don't worry about that. I'm inclined to say that your proposal here works and is fairly minimal, so safe to bring in (over some more convoluted lifetime management changes). I'm trying to dig in to the source of the button one. I've got this horrible theory that it's something that was fixed in Windows 11. I need to consult an expert here. You're on 10.0.19045.2728, so that at least would be quite a bit more up to date than my VM. I'm gonna fork that to another thread.
Author
Owner

@zadjii-msft commented on GitHub (May 18, 2023):

maybe another hint from elsewhere in the stack

Windows.UI.Xaml.dll!DirectUI::ToolTip::OpenPopup() Line 613	C++
Windows.UI.Xaml.dll!DirectUI::ToolTip::OnIsOpenChanged(unsigned char bIsOpen) Line 392	C++
Windows.UI.Xaml.dll!DirectUI::ToolTip::OnPropertyChanged2(const PropertyChangedParams & args) Line 244	C++
Windows.UI.Xaml.dll!DirectUI::DependencyObject::NotifyPropertyChanged(const PropertyChangedParams & args) Line 2342	C++
@zadjii-msft commented on GitHub (May 18, 2023): maybe another hint from elsewhere in the stack Windows.UI.Xaml.dll!DirectUI::ToolTip::OpenPopup() Line 613 C++ Windows.UI.Xaml.dll!DirectUI::ToolTip::OnIsOpenChanged(unsigned char bIsOpen) Line 392 C++ Windows.UI.Xaml.dll!DirectUI::ToolTip::OnPropertyChanged2(const PropertyChangedParams & args) Line 244 C++ Windows.UI.Xaml.dll!DirectUI::DependencyObject::NotifyPropertyChanged(const PropertyChangedParams & args) Line 2342 C++
Author
Owner

@zadjii-msft commented on GitHub (May 19, 2023):

I think closing the dialog here will always be useful, regardless of our solution to #15384. Let's PR that, and then work on a workaround for #15384 separately

@zadjii-msft commented on GitHub (May 19, 2023): I think closing the dialog here will always be useful, regardless of our solution to #15384. Let's PR that, and then work on a workaround for #15384 separately
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#19880