Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reported hang when minimized during a razzle build #14134

Closed
DHowett opened this issue Oct 4, 2022 · 14 comments
Closed

Reported hang when minimized during a razzle build #14134

DHowett opened this issue Oct 4, 2022 · 14 comments
Labels
Area-Windowing Window frame, quake mode, tearout Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Priority-0 Bugs that we consider release-blocking/recall-class (P0) Product-Terminal The new Windows Terminal. Resolution-Fix-Available It's available in an Insiders build or a release Severity-Blocking We won't ship a release like this! No-siree.

Comments

@DHowett
Copy link
Member

DHowett commented Oct 4, 2022

Windows Terminal version

1.16.2624

Steps to reproduce

  1. In razzle (I use psrazzle, so using powershell) start building
  2. Minimize Terminal (not sure why I do this, but I do this)

Actual Behavior

It hangs!

@DHowett DHowett added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Severity-Blocking We won't ship a release like this! No-siree. labels Oct 4, 2022
@carlos-zamora carlos-zamora added this to the Terminal v1.17 milestone Oct 5, 2022
@carlos-zamora carlos-zamora added Product-Terminal The new Windows Terminal. Priority-0 Bugs that we consider release-blocking/recall-class (P0) Area-Windowing Window frame, quake mode, tearout and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Oct 5, 2022
@stevenwdv
Copy link

stevenwdv commented Oct 18, 2022

Also for me WT regularly hangs when minimizing. But the specific conditions are not clear to me yet. However, I have no idea what razzle is so it would seem to me that the issue isn't specific to razzle; unless we're talking about different things. Sometimes when it hangs a tiny OpenConsole window becomes visible and when I close that WT works correctly again, not always though.

@vitek-karas
Copy link
Member

vitek-karas commented Oct 31, 2022

Can confirm the same behavior as @stevenwdv - for me this seems to happen often after the computer wakes up from sleep. It is happening so often that I'm hesitant touching the terminal window anymore.

If there's a way I'd be happy to provide crash dumps internally (or any other artifacts if you let me know how to collect those).

@DHowett
Copy link
Member Author

DHowett commented Oct 31, 2022

Thanks all for the confirming reports. I'd love any help you can give, so I followed up with you over e-mail. That having been said, if anyone in public has a hang dump or a crash dump to share I'd love to get it at the e-mail address on my profile.

@eternalphane
Copy link

@DHowett here you go (づ ̄ 3 ̄)づ

stack trace
For analysis of this file, run !analyze -v
win32u!NtUserMessageCall+0x14:
00007ff8`ac8b1434 c3              ret
0:000> ~*k

.  0  Id: 43e4.3828 Suspend: 0 Teb: 00000036`2b1b1000 Unfrozen
 # Child-SP          RetAddr               Call Site
00 00000036`2af2ebf8 00007ff8`ad6ae7a8     win32u!NtUserMessageCall+0x14
01 00000036`2af2ec00 00007ff8`ad6adf6f     user32!RealDefWindowProcWorker+0xe8
02 00000036`2af2ece0 00007ff8`a8aca210     user32!RealDefWindowProcW+0x4f
03 00000036`2af2ed20 00007ff8`a8ac7132     uxtheme!DoMsgDefault+0x38
04 00000036`2af2ed60 00007ff8`a8ad032e     uxtheme!OnDwpSysCommand+0x32
05 00000036`2af2ed90 00007ff8`a8acfc91     uxtheme!_ThemeDefWindowProc+0x68e
06 00000036`2af2efc0 00007ff8`ad6ae575     uxtheme!ThemeDefWindowProcW+0x11
07 00000036`2af2f000 00007ff6`1c9885ae     user32!DefWindowProcW+0x1f5
08 00000036`2af2f070 00007ff6`1c9882e6     WindowsTerminal+0x85ae
09 00000036`2af2f1b0 00007ff6`1c989bd7     WindowsTerminal+0x82e6
0a 00000036`2af2f290 00007ff8`ad6b1c4c     WindowsTerminal+0x9bd7
0b 00000036`2af2f310 00007ff8`ad6b0ea6     user32!UserCallWinProcCheckWow+0x33c
0c 00000036`2af2f480 00007ff6`1c989814     user32!DispatchMessageWorker+0x2a6
0d 00000036`2af2f500 00007ff6`1c98dfe2     WindowsTerminal+0x9814
0e 00000036`2af2f8f0 00007ff8`ae835550     WindowsTerminal+0xdfe2
0f 00000036`2af2f930 00007ff8`aeb0485b     kernel32!BaseThreadInitThunk+0x10
10 00000036`2af2f960 00000000`00000000     ntdll!RtlUserThreadStart+0x2b

   1  Id: 43e4.6840 Suspend: 0 Teb: 00000036`2b1c3000 Unfrozen
 # Child-SP          RetAddr               Call Site
00 00000036`2baff798 00007ff8`ac2cf180     ntdll!NtWaitForMultipleObjects+0x14

dump file

@zadjii-msft
Copy link
Member

from a mail thread

Terminal main thread is in uxtheme!ThemeDefWindowProcW and it’s just sitting there. That’s kinda weird.

Huh, 0xf028 isn’t a valid WM_SYSCOMMAND. 0xF020 is Minimize, which seems… close. Other code we have seems to only inspect wparam&0xfff0 anyways, so I suspect it was just minimize. That makes sense.

Over in the openconsole dump:

The main thread is trying to exit. Seems that the ConPTY output pipe was broken (_exitResult was -2147024664 coming from the WriteFile in VtEngine::_Flush). That triggers a
VtIo::_ShutdownIfNeeded
which leads to
RenderThread::WaitForPaintCompletionAndDisable

But, the renderer thread in openconsole is waiting for its own chance to paint, which it can’t, because the main thread is waiting for the renderer thread to paint before it releases the lock.

So that’s a deadlock, but that’s probably not the real issue here. The real issue is that somehow the pipe between the Terminal and the backing conpty broke? That straight up shouldn’t be possible. None of the three connection threads in the Terminal seem to show that, either. Something wack is up.

Hmm. This really looks like focus message pumping between the Terminal window and the “hidden” (heh) pseudoconsole windows that host every tab. There’s supposed to be one per tab …

@lhecker Weird question, but do you think your ConPTY shutdown changes have a chance of helping this case? If there’s lingering OpenConsole instances whose pseudowindows are parented to Terminal’s main window, and they’re deadlocked waiting for teardown, could that cause this?

OH that makes perfect sense. The Terminal’s main thread is blocked on its owned window (the conpty window)’s message pump processing whatever message the OS decided to send to it.

If Leonard’s PR doesn’t work, we could always try destroying the conpty window at the start of the teardown, before we get into any sort of deadlock scenario

@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Dec 1, 2022
@zadjii-msft zadjii-msft removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Dec 1, 2022
@DHowett
Copy link
Member Author

DHowett commented Dec 1, 2022

Huh, 0xf028 isn’t a valid WM_SYSCOMMAND. 0xF020 is Minimize, which seems… close. Other code we have seems to only inspect wparam&0xfff0 anyways, so I suspect it was just minimize. That makes sense.

FWIW the docs state:

In WM_SYSCOMMAND messages, the four low-order bits of the wParam parameter are used internally by the system.

😄

@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Dec 1, 2022
@lhecker
Copy link
Member

lhecker commented Dec 1, 2022

Yeah it should help with this a lot:

  • VtIo::_ShutdownIfNeeded doesn't lead to WaitForPaintCompletionAndDisable anymore. Instead it gracefully shuts down OpenConsole by sending all attached clients a CTRL_CLOSE_EVENT. Only once all clients have detached we call RundownAndExit which then calls WaitForPaintCompletionAndDisable. This used to cause most deadlocks I think, because RundownAndExit wasn't safe against reentrancy.
    It's async. --> It can't deadlock.
  • RundownAndExit is now reentrancy safe. Even though it's not strictly needed anymore, ever since VtIo::_ShutdownIfNeeded stopped calling RundownAndExit, this makes it safer against accidental recursion.
    No recursive PTY flush. --> No deadlock.
  • The first thing RundownAndExit now does is release the console lock.
    No lock. --> No deadlock.

Related PRs in order of creation:

@ghost ghost added the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Dec 6, 2022
@ghost

This comment was marked as resolved.

@DHowett DHowett removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. labels Dec 6, 2022
@DHowett
Copy link
Member Author

DHowett commented Dec 6, 2022

This is a known issue we're tracking; no reason for the bot to stomp it!

@zadjii-msft
Copy link
Member

Thanks for the writeup Leonard. We're gonna whip up a selfhost build next week. We should send that to {the internal folks that reported this originally} and make sure this is resolved.

@zadjii-msft
Copy link
Member

@vitek-karas Can you install 1.17 and see if that fixed this/?

@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jan 24, 2023
@zadjii-msft zadjii-msft added the Resolution-Fix-Available It's available in an Insiders build or a release label Jan 24, 2023
@zadjii-msft zadjii-msft removed this from the Terminal v1.17 milestone Jan 24, 2023
@zadjii-msft zadjii-msft added this to the Terminal v1.18 milestone Jan 24, 2023
@vitek-karas
Copy link
Member

@zadjii-msft I must admit I completely forgot about this - because I haven't seen it happen for a while now. I will upgrade to 1.17 and see what happens, but in the last 2 weeks (probably more, can't remember) I haven't seen it happen at all.

@zadjii-msft
Copy link
Member

Well, I'll take that for now and we can revisit later if this comes back. Thanks for confirming!

@stevenwdv
Copy link

stevenwdv commented Jan 25, 2023

For me seemed to happen mostly when I SSH'd into a remote Linux machine and opened some files with micro and then minimized the terminal (but not always), which I haven't done in a while. So for what it's worth, I also haven't ran into the problem for a while now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Windowing Window frame, quake mode, tearout Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Priority-0 Bugs that we consider release-blocking/recall-class (P0) Product-Terminal The new Windows Terminal. Resolution-Fix-Available It's available in an Insiders build or a release Severity-Blocking We won't ship a release like this! No-siree.
Projects
None yet
Development

No branches or pull requests

7 participants