nhyatt/putty-source
1
0
Fork 0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-10 01:48:00 +00:00
Code Issues Projects Releases Wiki Activity

Unix Plink: handle stdout/stderr backlog consistently.

Browse Source 
Whenever we successfully send some data to standard output/error,
we're supposed to notify the backend that this has happened, and tell
it how much backlog still remains, by calling backend_unthrottle().

In Unix Plink, the call to backend_unthrottle() was happening on some
but not all calls to try_output(). In particular, it was happening
when we called try_output() as a result of stdout or stderr having
just been reported writable by poll(), but not when we called it from
plink_output() after the backend had just sent us some more data. Of
course that _normally_ works - if you were polling stdout for
writability at all then it's because a previous call had returned
EAGAIN, so that's when you _have_ backlog to dispose of. But it's also
possible, by an accident of timing, that before you get round to doing
that poll, the seat passes you further data and you call try_output()
anyway, and by chance, the blockage has cleared. In that situation,
you end up having cleared your backlog but forgotten to tell the
backend about it - which might mean the backend never unfreezes the
channel or (in 'simple' mode) the entire SSH socket.

A user reported (and I reproduced) that when Plink is compiled on
MacOS, running an interactive session through it and doing
output-intensive activity like scrolling around in htop(1) can quite
easily get it into what turned out to be that stuck state. (I don't
know why MacOS and not any other platform, but since it's a race
condition, that seems like a plausible enough cause of a difference in
timing.)

Also, we were inconsistently computing the backlog size: sometimes it
was the total size of the stdout and stderr bufchains, and sometimes
it was just the size of the one we'd made an effort to empty.

Now the backlog size is consistently stdout+stderr (the same as it is
in Windows Plink), and the call to backend_unthrottle() happens
_inside_ try_output(), so that I don't have to remember it at every
call site.
This commit is contained in:
Simon Tatham 2022-07-21 18:37:58 +01:00
parent 42740a5455
commit 810e21de82
1 changed files with 14 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
unix/plink.c
View File

@ -322,7 +322,12 @@ static BinarySink *stdout_bs, *stderr_bs;
static enum { EOF_NO, EOF_PENDING, EOF_SENT } outgoingeof; static enum { EOF_NO, EOF_PENDING, EOF_SENT } outgoingeof;
size_t try_output(bool is_stderr) static size_t output_backlog(void)
{
return bufchain_size(&stdout_data) + bufchain_size(&stderr_data);
}
void try_output(bool is_stderr)
{ {
bufchain *chain = (is_stderr ? &stderr_data : &stdout_data); bufchain *chain = (is_stderr ? &stderr_data : &stdout_data);
int fd = (is_stderr ? STDERR_FILENO : STDOUT_FILENO); int fd = (is_stderr ? STDERR_FILENO : STDOUT_FILENO);
@ -343,12 +348,13 @@ size_t try_output(bool is_stderr)
perror(is_stderr ? "stderr: write" : "stdout: write"); perror(is_stderr ? "stderr: write" : "stdout: write");
exit(1); exit(1);
} }
backend_unthrottle(backend, output_backlog());
} }
if (outgoingeof == EOF_PENDING && bufchain_size(&stdout_data) == 0) { if (outgoingeof == EOF_PENDING && bufchain_size(&stdout_data) == 0) {
close(STDOUT_FILENO); close(STDOUT_FILENO);
outgoingeof = EOF_SENT; outgoingeof = EOF_SENT;
} }
return bufchain_size(&stdout_data) + bufchain_size(&stderr_data);
} }
static size_t plink_output( static size_t plink_output(
@ -360,7 +366,8 @@ static size_t plink_output(
BinarySink *bs = is_stderr ? stderr_bs : stdout_bs; BinarySink *bs = is_stderr ? stderr_bs : stdout_bs;
put_data(bs, data, len); put_data(bs, data, len);
return try_output(is_stderr); try_output(is_stderr);
return output_backlog();
} }
static bool plink_eof(Seat *seat) static bool plink_eof(Seat *seat)
@ -650,13 +657,11 @@ static void plink_pw_check(void *vctx, pollwrapper *pw)
} }
} }
if (pollwrap_check_fd_rwx(pw, STDOUT_FILENO, SELECT_W)) { if (pollwrap_check_fd_rwx(pw, STDOUT_FILENO, SELECT_W))
backend_unthrottle(backend, try_output(false)); try_output(false);
}
if (pollwrap_check_fd_rwx(pw, STDERR_FILENO, SELECT_W)) { if (pollwrap_check_fd_rwx(pw, STDERR_FILENO, SELECT_W))
backend_unthrottle(backend, try_output(true)); try_output(true);
}
} }
static bool plink_continue(void *vctx, bool found_any_fd, static bool plink_continue(void *vctx, bool found_any_fd,