-
Notifications
You must be signed in to change notification settings - Fork 317
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
Prevent duplicated login message #426
Prevent duplicated login message #426
Conversation
Entering raw mode during an interactive terminal session (like using a remote shell) can print out the login message twice. This is caused by a WINDOW_BUFFER_SIZE_EVENT (that we caused by changing console settings) to be translated into a SIGWINCH, which is not handled, and thus treated as an error by the write thread, after already successfully writing to the console.
@@ -221,6 +221,17 @@ ConEnterRawMode() | |||
ScrollBottom = ConVisibleWindowHeight(); | |||
|
|||
in_raw_mode = 1; | |||
|
|||
/* | |||
Consume and ignore the first WINDOW_BUFFER_SIZE_EVENT, as we've triggered it ourselves by updating the console settings above. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you help me to understand the scenario better?
I had your code change locally. I didn't hit this code. It always hits WINDOW_BUFFER_SIZE_EVENT in ReadConsoleForTermEmul() in tncon.c.
- When do we hit this code?
- Why will the SIGWINCH interrupt duplicate the login message? SIGWINCH handler sends the window size information but not write anything to console?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to apologize in advance for the long message, but I figure too much context is better than not enough. So..
I can hit this code while running openssh from either command prompt or powershell. I am passing following flags/args, and also note that the remote machine is running linux:
-p <port_number> -i "<id_rsa>" -oStrictHostKeyChecking=yes -oUserKnownHostsFile="<known_hosts>" <user>@<ip_address>
The specific issue occurs in fileio_write() in fileio.c
the code that waits for the asynchronous writing using the WriteThread does not finish before wait_for_any_event() gets called and returns that there was a signal that happened while writing -- that signal being the SIGWINCH created by the WINDOW_BUFFER_SIZE_EVENT we made by changing the console handle's settings.
if (w32_io_is_blocking(pio)) {
while (pio->write_details.pending) {
if (wait_for_any_event(NULL, 0, INFINITE) == -1) {
/* if interrupted but write has completed, we are good*/
if ((errno != EINTR) || (pio->write_details.pending))
return -1;
errno = 0;
}
}
}
Because the SIGWINCH is treated as a not-handled interrupt, the call to fileio_write "fails" although the asynchronous WriteThread successfully writes to the screen.
In my case, ssh happens to get new data from the remote shell, typically the prompt for input looking something like:
<user>@<host>:<pwd>$
And because it has received new data to output, but the buffer has not been cleared because it was "not written" since the call to fileio_write failed (as noted above), it prints the first part (the login message for me) twice, but the second part (the prompt for input) only once.
Entering raw mode during an interactive terminal session (like using a
remote shell) can print out the login message twice. This is caused by a
WINDOW_BUFFER_SIZE_EVENT (that we caused by changing console settings)
to be translated into a SIGWINCH, which is not handled, and thus treated
as an error by the write thread, after already successfully writing to
the console.