Patch from Alan Ning (somewhat polished by me): introduce a flag
authorsimon <simon@cda61777-01e9-0310-a592-d414129be87e>
Sun, 4 Jul 2010 22:53:53 +0000 (22:53 +0000)
committersimon <simon@cda61777-01e9-0310-a592-d414129be87e>
Sun, 4 Jul 2010 22:53:53 +0000 (22:53 +0000)
called 'pending_close'. This deals with the situation in which we're
forwarding a port, have received and locally buffered some data from
the local endpoint but not yet been able to pass it down the SSH
connection due to window limitations, and then the local endpoint
closes its socket. In this situation what we've been doing until now
is to immediately send SSH2_MSG_CHANNEL_CLOSE, causing the data
still in our local buffer to be lost; now we instead set the new
flag, which will remind us to send SSH2_MSG_CHANNEL_CLOSE _after_ we
empty our bufchain.

Should fix at least one manifestation of 'portfwd-close', though I
don't know if it's the cause of all the reports we've ever seen.

git-svn-id: svn://svn.tartarus.org/sgt/putty@8971 cda61777-01e9-0310-a592-d414129be87e

ssh.c

diff --git a/ssh.c b/ssh.c
index 0616eff..8208246 100644 (file)
--- a/ssh.c
+++ b/ssh.c
@@ -590,6 +590,17 @@ struct ssh_channel {
      * A channel is completely finished with when all four bits are set.
      */
     int closes;
+
+    /*
+     * This flag indicates that a close is pending on the outgoing
+     * side of the channel: that is, wherever we're getting the data
+     * for this channel has sent us some data followed by EOF. We
+     * can't actually close the channel until we've finished sending
+     * the data, so we set this flag instead to remind us to
+     * initiate the closing process once our buffer is clear.
+     */
+    int pending_close;
+
     /*
      * True if this channel is causing the underlying connection to be
      * throttled.
@@ -4165,14 +4176,42 @@ void sshfwd_close(struct ssh_channel *c)
            if (ssh->version == 1) {
                send_packet(ssh, SSH1_MSG_CHANNEL_CLOSE, PKT_INT, c->remoteid,
                            PKT_END);
+               c->closes = 1;                 /* sent MSG_CLOSE */
            } else {
-               struct Packet *pktout;
-               pktout = ssh2_pkt_init(SSH2_MSG_CHANNEL_CLOSE);
-               ssh2_pkt_adduint32(pktout, c->remoteid);
-               ssh2_pkt_send(ssh, pktout);
+               int bytes_to_send = bufchain_size(&c->v.v2.outbuffer);
+               if (bytes_to_send > 0) {
+                   /*
+                    * If we still have unsent data in our outgoing
+                    * buffer for this channel, we can't actually
+                    * initiate a close operation yet or that data
+                    * will be lost. Instead, set the pending_close
+                    * flag so that when we do clear the buffer
+                    * we'll start closing the channel.
+                    */
+                   char logmsg[160] = {'\0'};
+                   sprintf(
+                           logmsg,
+                           "Forwarded port pending to be closed : "
+                           "%d bytes remaining",
+                           bytes_to_send);
+                   logevent(logmsg);
+
+                   c->pending_close = TRUE;
+               } else {
+                   /*
+                    * No locally buffered data, so we can send the
+                    * close message immediately.
+                    */
+                   struct Packet *pktout;
+                   pktout = ssh2_pkt_init(SSH2_MSG_CHANNEL_CLOSE);
+                   ssh2_pkt_adduint32(pktout, c->remoteid);
+                   ssh2_pkt_send(ssh, pktout);
+                   c->closes = 1;                     /* sent MSG_CLOSE */
+                   logevent("Nothing left to send, closing channel");
+               }
            }
        }
-       c->closes = 1;                 /* sent MSG_CLOSE */
+
        if (c->type == CHAN_X11) {
            c->u.x11.s = NULL;
            logevent("Forwarded X11 connection terminated");
@@ -4696,6 +4735,7 @@ static void ssh1_smsg_x11_open(Ssh ssh, struct Packet *pktin)
            c->halfopen = FALSE;
            c->localid = alloc_channel_id(ssh);
            c->closes = 0;
+           c->pending_close = FALSE;
            c->throttling_conn = 0;
            c->type = CHAN_X11; /* identify channel type */
            add234(ssh->channels, c);
@@ -4725,6 +4765,7 @@ static void ssh1_smsg_agent_open(Ssh ssh, struct Packet *pktin)
        c->halfopen = FALSE;
        c->localid = alloc_channel_id(ssh);
        c->closes = 0;
+       c->pending_close = FALSE;
        c->throttling_conn = 0;
        c->type = CHAN_AGENT;   /* identify channel type */
        c->u.a.lensofar = 0;
@@ -4779,6 +4820,7 @@ static void ssh1_msg_port_open(Ssh ssh, struct Packet *pktin)
            c->halfopen = FALSE;
            c->localid = alloc_channel_id(ssh);
            c->closes = 0;
+           c->pending_close = FALSE;
            c->throttling_conn = 0;
            c->type = CHAN_SOCKDATA;    /* identify channel type */
            add234(ssh->channels, c);
@@ -6345,7 +6387,7 @@ static int ssh2_try_send(struct ssh_channel *c)
     return bufchain_size(&c->v.v2.outbuffer);
 }
 
-static void ssh2_try_send_and_unthrottle(struct ssh_channel *c)
+static void ssh2_try_send_and_unthrottle(Ssh ssh, struct ssh_channel *c)
 {
     int bufsize;
     if (c->closes)
@@ -6369,6 +6411,19 @@ static void ssh2_try_send_and_unthrottle(struct ssh_channel *c)
            break;
        }
     }
+
+    /*
+     * If we've emptied the channel's output buffer and there's a
+     * pending close event, start the channel-closing procedure.
+     */
+    if (c->pending_close && bufchain_size(&c->v.v2.outbuffer) == 0) {
+       struct Packet *pktout;
+       pktout = ssh2_pkt_init(SSH2_MSG_CHANNEL_CLOSE);
+       ssh2_pkt_adduint32(pktout, c->remoteid);
+       ssh2_pkt_send(ssh, pktout);
+       c->closes = 1;
+       c->pending_close = FALSE;
+    }
 }
 
 /*
@@ -6379,6 +6434,7 @@ static void ssh2_channel_init(struct ssh_channel *c)
     Ssh ssh = c->ssh;
     c->localid = alloc_channel_id(ssh);
     c->closes = 0;
+    c->pending_close = FALSE;
     c->throttling_conn = FALSE;
     c->v.v2.locwindow = c->v.v2.locmaxwin = c->v.v2.remlocwin =
        ssh->cfg.ssh_simple ? OUR_V2_BIGWIN : OUR_V2_WINSIZE;
@@ -6562,7 +6618,7 @@ static void ssh2_msg_channel_window_adjust(Ssh ssh, struct Packet *pktin)
        return;
     if (!c->closes) {
        c->v.v2.remwindow += ssh_pkt_getuint32(pktin);
-       ssh2_try_send_and_unthrottle(c);
+       ssh2_try_send_and_unthrottle(ssh, c);
     }
 }
 
@@ -8936,7 +8992,7 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen,
             * Try to send data on all channels if we can.
             */
            for (i = 0; NULL != (c = index234(ssh->channels, i)); i++)
-               ssh2_try_send_and_unthrottle(c);
+               ssh2_try_send_and_unthrottle(ssh, c);
        }
     }