1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-09 17:38:00 +00:00

Work around unhelpful GTK event ordering.

If the SSH socket is readable, GTK will preferentially give us a
callback to read from it rather than calling its idle functions. That
means the ssh->in_raw bufchain can just keep accumulating data, and
the callback that gets the BPP to take data back off that bufchain
will never be called at all.

The solution is to use sk_set_frozen after a certain point, to stop
reading further data from the socket (and, more importantly, disable
GTK's I/O callback for that fd) until we've had a chance to process
some backlog, and then unfreeze the socket again afterwards.

Annoyingly, that means adding a _second_ 'frozen' flag to Ssh, because
the one we already had has exactly the wrong semantics - it prevents
us from _processing_ our backlog, which is the last thing we want if
the entire problem is that we need that backlog to get smaller! So now
there are two frozen flags, and a big comment explaining the
difference.
This commit is contained in:
Simon Tatham 2019-02-06 06:52:25 +00:00
parent 26beafe984
commit 0f405ae8a3
4 changed files with 67 additions and 13 deletions

66
ssh.c
View File

@ -63,7 +63,29 @@ struct Ssh {
int conn_throttle_count;
int overall_bufsize;
bool throttled_all;
bool frozen;
/*
* logically_frozen is true if we're not currently _processing_
* data from the SSH socket (e.g. because a higher layer has asked
* us not to due to ssh_throttle_conn). socket_frozen is true if
* we're not even _reading_ data from the socket (i.e. it should
* always match the value we last passed to sk_set_frozen).
*
* The two differ in that socket_frozen can also become
* temporarily true because of a large backlog in the in_raw
* bufchain, to force no further plug_receive events until the BPP
* input function has had a chance to run. (Some front ends, like
* GTK, can persistently call the network and never get round to
* the toplevel callbacks.) If we've stopped reading from the
* socket for that reason, we absolutely _do_ want to carry on
* processing our input bufchain, because that's the only way
* it'll ever get cleared!
*
* ssh_check_frozen() resets socket_frozen, and should be called
* whenever either of logically_frozen and the bufchain size
* changes.
*/
bool logically_frozen, socket_frozen;
/* in case we find these out before we have a ConnectionLayer to tell */
int term_width, term_height;
@ -295,6 +317,29 @@ static void ssh_got_ssh_version(struct ssh_version_receiver *rcv,
ssh_bpp_free(old_bpp);
}
static void ssh_check_frozen(Ssh *ssh)
{
if (!ssh->s)
return;
bool prev_frozen = ssh->socket_frozen;
ssh->socket_frozen = (ssh->logically_frozen ||
bufchain_size(&ssh->in_raw) > SSH_MAX_BACKLOG);
sk_set_frozen(ssh->s, ssh->socket_frozen);
if (prev_frozen && !ssh->socket_frozen && ssh->bpp) {
/*
* If we've just unfrozen, process any SSH connection data
* that was stashed in our queue while we were frozen.
*/
queue_idempotent_callback(&ssh->bpp->ic_in_raw);
}
}
void ssh_conn_processed_data(Ssh *ssh)
{
ssh_check_frozen(ssh);
}
static void ssh_bpp_output_raw_data_callback(void *vctx)
{
Ssh *ssh = (Ssh *)vctx;
@ -321,6 +366,8 @@ static void ssh_bpp_output_raw_data_callback(void *vctx)
}
}
ssh_check_frozen(ssh);
if (ssh->pending_close) {
sk_close(ssh->s);
ssh->s = NULL;
@ -536,8 +583,10 @@ static void ssh_receive(Plug *plug, int urgent, char *data, int len)
0, NULL, NULL, 0, NULL);
bufchain_add(&ssh->in_raw, data, len);
if (!ssh->frozen && ssh->bpp)
if (!ssh->logically_frozen && ssh->bpp)
queue_idempotent_callback(&ssh->bpp->ic_in_raw);
ssh_check_frozen(ssh);
}
static void ssh_sent(Plug *plug, int bufsize)
@ -753,17 +802,8 @@ void ssh_throttle_conn(Ssh *ssh, int adjust)
return; /* don't change current frozen state */
}
ssh->frozen = frozen;
if (ssh->s) {
sk_set_frozen(ssh->s, frozen);
/*
* Now process any SSH connection data that was stashed in our
* queue while we were frozen.
*/
queue_idempotent_callback(&ssh->bpp->ic_in_raw);
}
ssh->logically_frozen = frozen;
ssh_check_frozen(ssh);
}
/*

3
ssh.h
View File

@ -378,6 +378,9 @@ void ssh_got_exitcode(Ssh *ssh, int status);
void ssh_ldisc_update(Ssh *ssh);
void ssh_got_fallback_cmd(Ssh *ssh);
/* Communications back to ssh.c from the BPP */
void ssh_conn_processed_data(Ssh *ssh);
/* Functions to abort the connection, for various reasons. */
void ssh_remote_error(Ssh *ssh, const char *fmt, ...);
void ssh_remote_eof(Ssh *ssh, const char *fmt, ...);

View File

@ -822,7 +822,11 @@ void ssh_ppl_user_output_string_and_free(PacketProtocolLayer *ppl, char *text)
static void ssh_bpp_input_raw_data_callback(void *context)
{
BinaryPacketProtocol *bpp = (BinaryPacketProtocol *)context;
Ssh *ssh = bpp->ssh; /* in case bpp is about to get freed */
ssh_bpp_handle_input(bpp);
/* If we've now cleared enough backlog on the input connection, we
* may need to unfreeze it. */
ssh_conn_processed_data(ssh);
}
static void ssh_bpp_output_packet_callback(void *context)

View File

@ -201,6 +201,13 @@ void ssh_throttle_conn(Ssh *ssh, int adjust)
}
}
void ssh_conn_processed_data(Ssh *ssh)
{
/* FIXME: we could add the same check_frozen_state system as we
* have in ssh.c, but because that was originally added to work
* around a peculiarity of the GUI event loop, I haven't yet. */
}
static const PlugVtable ssh_server_plugvt = {
server_socket_log,
server_closing,