microsoft
Hidden cursors sometimes reappear unexpectedly
Windows Terminal version
1.18.1421.0
Windows build number
10.0.19045.2913
Other Software
No response
Steps to reproduce
I don't have a simple test case to reproduce this yet, but what I'm doing is playing a terminal game that hides the cursor on startup.
Expected Behavior
I shouldn't be able to see the cursor while the game is in progress.
Actual Behavior
9 times out of 10, the cursor is visible.
Having added some logging in AdaptDispatch, I can see that conhost is receiving a single DECTCEM sequence to hide the cursor, but what Windows Terminal ends up receiving is two requests to hide the cursor, followed by another request to show it again. I'm guessing this is the result of some conpty reordering that has gone wrong.
A git bisect suggests that this might be a regression caused by PR #14677. I'm not absolutely certain of that though, because the issue doesn't always reproduce 100% of the time.
I'll try and come up with a simple test case when I have more time, but I just wanted to get the issue filed before I forget about it.
I think this was fixed by PR #15991. Prior to commit 41f7ed73c1e62195919146f689135517aefbaf7b I could reproduce the problem quite easily, but now it seems to be fine. I'm happy for this issue to be closed.
@lhecker I've noticed that this bug is back again. I can reproduce in 1.20.10822.0 and the latest commit to main. A git bisect suggests it regressed in bdf2f6f2740168363bdba898697e28baa8d7beb7, but I suspect now that it may be timing related, and the "fix" in 41f7ed73c1e62195919146f689135517aefbaf7b and "regression" in bdf2f6f2740168363bdba898697e28baa8d7beb7 are just because the flushing affects the timing. So it's possible the bug was still there the whole time, and that commit isn't really to blame. I still don't have a simple test case, but I'm going to reopen the issue in the meantime so I don't forget about it.
Oww, that's unfortunate to hear. 😕 My series of PRs around the flushing of VtEngine really isn't particularly great, and I'm fairly dissatisfied with them. I couldn't think of an alternative approach to make it robust with any unknown escape sequence though and I kind of still can't. Maybe I'm missing something obvious.
I'm still eager to get rid of VtEngine entirely however [^1], and I'm hoping to get it all done at some point within a year from now.
[^1]: I mentioned this before elsewhere, but what I mean is my idea of an algorithmic console API -> VT translation, and by using the ConPTY TextBuffers only for serving the ReadConsole* APIs. That'll certainly result in bugs (when it goes out of sync with the terminal), but I suspect that the total number of bugs then would still be less than the total number of bugs we have now, especially since it goes out of sync right now as well.
I finally figured out the source of the problem. It's this code: https://github.com/microsoft/terminal/blob/378b6594bd94cf3b27f4e309a11efe25d83de0d9/src/renderer/vt/XtermEngine.cpp#L111-L117
If _noFlushOnEnd is set, the buffer won't necessarily be flushed at the end of every frame, so when the insert happens, we could have multiple frames pending in the buffer. This means the HideCursor sequence is inserted at the start of the wrong frame, and in my case it was being inserted before the ShowCursor sequence that it should have been cancelling.
It can easily be fixed just by tracking the buffer size at the start of the frame, and using that saved offset for the insert position here.