Guard IslandWindow shutdown against exploding? #19322
Conversation
During shutdown (specifically from the right-click menu in the terminal) XAML would throw an exception that we traced back to resetting the content of the Island. ``` Exception thrown at 0x00007FF95E72B5EC (KernelBase.dll) in WindowsTerminal.exe: WinRT originate error - 0x800F1000 : 'Child collection must not be modified during measure or arrange.'. ``` This made it out into the catch around the window message handler and resulted in the window not being closed after being emptied out. Closes #19312
There was a problem hiding this comment.
I find the error message to be a little sus. Is it possible that we're in some nested call stack when pumping the message loop?
I'm asking because if those throws I suppose the cleanup of XAML failed and so we got a resource leak.
In that case we could try closing windows asynchronously by queuing a Low priority coroutine.
|
@lhecker honestly, i don't know. i didn't see that we were in a nested call stack. I do think that complicating shutdown/window closure to make it asynchronous is going to come back to bite us (because we will invariably have windows left around in various states of closure when the process goes down, and I don't know what that's going to do) |
|
I can confirm the stack is as shallow as it gets. There's practically no chance that what XAML says is happening is happening. It is not being modified in the middle of a Measure/Layout pass because it has ALREADY been serialized onto the main UI thread. |
|
|
I was thinking something like this: https://github.com/microsoft/terminal/tree/dev/lhecker/wellp2-alt |
|
Sure, but doesn't that accomplish the same thing as posting the window message to begin with? Doesn't it wait for the next time around the loop to do a Dispatch Queue run? |
|
No, I don't think so. You can't specify a priority with PostMessage. As we've seen in previous issues, WinUI seems very picky about needing specifically low-priority messages to work around things like "UI elements get no assigned size whatsoever otherwise". Given that your exception is "Child collection must not be modified during measure" I had the suspicion that this is the same thing. |
|
Tested your fix and mine. Yours doesn't fix it, but neither does mine! With yours, we still get the issue. There's a delayed layout pass happening somewhere. With mine, with 2 windows, we sure don't crash on the exception and we ABSOLUTELY do crash later when deep inside XAML a |
During shutdown (specifically from the right-click menu in the terminal) XAML would throw an exception that we traced back to resetting the content of the Island.
This made it out into the catch around the window message handler and resulted in the window not being closed after being emptied out.
Closes #19312