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

Sometimes windows don't get persisted on a Windows Update #17179

Open
zadjii-msft opened this issue May 2, 2024 · 10 comments
Open

Sometimes windows don't get persisted on a Windows Update #17179

zadjii-msft opened this issue May 2, 2024 · 10 comments
Labels
Area-Windowing Window frame, quake mode, tearout Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal.

Comments

@zadjii-msft
Copy link
Member

v1.21

I've now had a couple days where I've gotten a Windows Update, and came back to the Terminal not quite in the right state as before:

  • A few times, I've had the buffer restored, but to a previous day's state. As if the buffer didn't get persisted on shutdown, but on launch I did get the buffer from the previous shutdown.
  • Today, I only had one of my two windows persisted across the reboot. The window that was persisted did have all it's buffers restored, but the other window was gone entirely.

I have a theory that this is somehow related to us not handling WM_ENDSESSION post-PMv3 quite right, but that's a guess, not a root cause.

@zadjii-msft zadjii-msft added Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. labels May 2, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels May 2, 2024

This comment was marked as off-topic.

@zadjii-msft
Copy link
Member Author

Yea, again today, I got both windows back, but the buffer state was what it was the previous time I had quit the Terminal. So, whatever happened during the update meant that the Terminal didn't actually persist the buffer.

@carlos-zamora carlos-zamora added this to the Terminal v1.22 milestone May 8, 2024
@carlos-zamora carlos-zamora added 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 Needs-Tag-Fix Doesn't match tag requirements labels May 8, 2024
@lhecker
Copy link
Member

lhecker commented May 13, 2024

I found that this tool can be used for shutdown tests:

C:\Program Files (x86)\Windows Kits\10\App Certification Kit\rmlogotest.exe

For instance:

& "C:\Program Files (x86)\Windows Kits\10\App Certification Kit\rmlogotest.exe" (ps -Name WindowsTerminal).Id

I couldn't reproduce any issues on the main branch, even with multiple windows that have multiple tabs, and no matter how I re-launched the app.

@lhecker
Copy link
Member

lhecker commented May 14, 2024

tl;dr: handoff doesn't assign a SessionId!

I figured out how to reproduce multiple issues (probably even the issue):

  • Launch
  • (Register that version of WT as the default)
  • Close all tabs (Ctrl+Shift+W)
  • Bug: persistedWindowLayouts is non-empty
  • Launch cmd/pwsh via handoff, but use the one that's not your default profile
  • (Known) Bug: You got 2 windows now instead of 1 (this is a consequence of the previous bug)
  • Close the tab with the default profile (= the extra window)
  • Close window with the handoff'd non-default profile (= press red X)
  • Bug: Buffer doesn't get persisted, and no SessionId gets stored

@zadjii-msft
Copy link
Member Author

So yea here's the thing - defterm wasn't involved (to the best of my knowledge) in my scenario. I almost never open things via defterm anymore - pretty sure Stable is set as my defterm handler. Normally quitting the terminal will persist just fine - there's something specific about updates that makes the terminal not persist the current state of the buffer 🤷

@mmseng
Copy link

mmseng commented Jun 28, 2024

I don't know if this belongs here, but I've been having a hell of a time getting Terminal to save my buffer sessions. It works fine when I manually close Terminal. However pretty much the only reasons Terminal ever gets restarted in my experience is A) Windows updates or B) system crashes (yes I have some system issue that occasionally causes crashes).

What I've noticed is that I can easily recreate the issue by simply killing the WindowsTerminal.exe or OpenConsole.exe processes. Notably, the behavior is different depending on which one is killed.

Killing WindowsTerminal.exe simply fails to save any new buffer content to the existing buffer dump text file(s). Reopening Terminal restores the content in those files to the appropriate tabs, but any new buffer content during the previous run wasn't captured, so the content restored is incomplete. zadjii noted this behavior in the OP.

However, killing OpenConsole.exe has far worse behavior. Killing all instances of OpenConsole.exe (apparently one for each tab) completely blows away (read: deletes) all buffer text files entirely. When reopening Terminal, not even the tabs are restored, and the one fresh tab that is created has no buffer content, as you would expect.

This is the behavior I'm seeing every time I come back to my machine after a Windows update or a system crash. No tabs are restored, no buffer content is restored, and no buffer dump files exist. I have to assume that the restarts being performed even by successful Windows updates are not gracefully closing Terminal. So the new buffer functionality has been largely useless for me so far.

If there were some way to force Terminal to update the buffer file on a schedule, before closing Terminal, this might be more useful, but even then, if it randomly decides to delete the buffer files when OpenConsole is force closed, then even that is not a solution.

I considered implementing a custom solution which monitors my buffer dump files and backs them up, however even that wouldn't be useful, since Terminal doesn't actually update those files until it is gracefully closed. Personally, I would happily eat whatever performance hit would be required for Terminal to update my buffer dump files on a frequent, and/or configurable schedule (and of course not delete them when force closed). I'm sure that's not a universal opinion.

@lhecker
Copy link
Member

lhecker commented Jun 28, 2024

Killing all instances of OpenConsole.exe (apparently one for each tab) completely blows away (read: deletes) all buffer text files entirely.

We could check OpenConsole's exit code and see if it's neither 0 nor STILL_ACTIVE.

More importantly, I wonder if this is finally the clue as to why the buffer contents are lost on a windows update. Does the OS perhaps kill OpenConsole before it sends a termination message to Windows Terminal? That would definitely be a problem.
As I said above, I tried reproducing the issue with rmlogotest.exe and it worked perfectly, so theoretically the implementation inside Windows Terminal should be correct.

@mmseng
Copy link

mmseng commented Jun 28, 2024

Just for the record, I noticed that Terminal has a couple of other child processes, but I didn't get around to testing what happens when I kill those. Once I saw the behavior of killing OpenConsole.exe I called it quits.

Also, I will add that by "killing processes" I mean using Task Manager's "Details" tab, right clicking and selecting "End Task".

Notably, if I use the "Processes" tab and do the same, it actually seems to send a termination message to the processes instead, and the resulting behavior is not the same. So this is important if trying to reproduce the behavior by killing processes with Task Manager.

@lhecker
Copy link
Member

lhecker commented Jun 28, 2024

The other child processes of Windows Terminal are the shells and console applications you spawn.

By the way, when I said that we could check the exit code, I would not consider that an important "bug fix" or similar, but more like a nicety. OpenConsole is effectively the same as conhost and for both processes killing them is basically analogous to killing any shell or console applications they host. That's definitely way worse than not persisting the buffer contents, because that console application may have been a critical one. I'm sure they don't build rockets with Windows, but I wouldn't want to make assumptions what kind of applications people build with this OS. For the same reason, neither OpenConsole nor conhost are meant to ever crash and I think it's incredibly rare that they do.

So, in that sense, killing OpenConsole as proof that we don't persist buffer contents correctly is IMO not fair. That's no different from killing Firefox or Chrome child processes, which also loses you all state, depending on when they got last persisted.

However, killing Windows Terminal and it not having persisted any buffer contents whatsoever, that's IMO totally fair. Browsers do it, so we should probably do it too. We can fix that by regularly backing up the buffer contents as you suggested. I'm just not 100% sure when to do that, because the buffer serialization is not yet performance optimized and freezes the application for a noticeable amount of time when it runs with 10+ tabs.

@mmseng
Copy link

mmseng commented Jun 28, 2024

Sure. I was not accusing any component of working incorrectly. Just providing some insights into what I see actually happening.

I appreciate hearing that backing up buffer contents more regularly is at least an agreeable idea. How and when is of course another matter that I have not the expertise to comment on.

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. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

4 participants