Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Port 'Delay console CPR timer once protocol has worked' to 2.1#30789

Merged
danmoseley merged 1 commit into
dotnet:release/2.1from
wtgodbe:CprTimer21
Jul 3, 2018
Merged

Port 'Delay console CPR timer once protocol has worked' to 2.1#30789
danmoseley merged 1 commit into
dotnet:release/2.1from
wtgodbe:CprTimer21

Conversation

@wtgodbe
Copy link
Copy Markdown
Member

@wtgodbe wtgodbe commented Jul 2, 2018

Port of #30674 for 2.1

Copied from the other PR:

Getting the current cursor position involves writing a particular escape sequence to stdout and then reading/parsing the response. In case the terminal doesn't support the sequence, we use tcsetattr to tell the terminal set set c_cc[MIN] to 0 and c_cc[VTIME] to 10; this makes it so that if no input is received within one second, we'll give up on reading the response. If we didn't do this and no response came back, or if a faulty response came back such that we would otherwise just sit there waiting for something that would never come, the console could end up hanging until the user typed the expected sequence.

However, this timeout can cause problems when the terminal is separated by a slow network connection from the console logic, in which case the CPR can time out and the response can end up not only being echoed to the screen, but also interpreted as user input.

This change tracks whether we've ever received a successful CPR response. Until we have, it keeps using 0 for c_cc[MIN], i.e. the minimum number of chars we need to read, such that it relies on the timeout firing and we return when either we've read at least one byte or the timer fires. Once we've successfully read a CPR response, for all future requests, we set c_cc[MIN] to 1. With that, we'll still return once at least one byte is received, but the timer won't start until then, which helps avoid this issue with slow network connections.

For issue #30406 for the powershell team

CC @stephentoub @jianyunt @edyoung

The hope is to get this in for the 2.1.3 release

Copy link
Copy Markdown
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@danmoseley
Copy link
Copy Markdown
Member

Approved

@danmoseley danmoseley merged commit 59b1d9a into dotnet:release/2.1 Jul 3, 2018
@karelz karelz added this to the 2.1.x milestone Jul 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants