Hidden cursors sometimes reappear unexpectedly

[j4james]

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.

[j4james]

I think this was fixed by PR #15991. Prior to commit 41f7ed7 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'll be closing this issue then. 🙂

[j4james]

@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 bdf2f6f, but I suspect now that it may be timing related, and the "fix" in 41f7ed7 and "regression" in bdf2f6f 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.

[lhecker]

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 ¹, and I'm hoping to get it all done at some point within a year from now.

Footnotes

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. ↩

[j4james]

I finally figured out the source of the problem. It's this code:

terminal/src/renderer/vt/XtermEngine.cpp

Lines 111 to 117 in 378b659

	// If the cursor was previously visible, let's hide it for this frame,
	// by prepending a cursor off.
	if (_lastCursorIsVisible != Tribool::False)
	{
	_buffer.insert(0, "\x1b[?25l");
	_lastCursorIsVisible = Tribool::False;
	}

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.