fixed sftp_shutdown() for the blockiness and fixed libssh2_channel_close()

to work properly non-blocking

src/channel.c

@@ -1,4 +1,4 @@
-/* Copyright (c) 2004-2007, Sara Golemon <sarag@libssh2.org>
+/* Copyright (c) 2004-2007 Sara Golemon <sarag@libssh2.org>
  * Copyright (c) 2008-2009 by Daniel Stenberg
  *
  * All rights reserved.
@@ -2302,9 +2302,6 @@ channel_close(LIBSSH2_CHANNEL * channel)
         _libssh2_debug(session, LIBSSH2_DBG_CONN, "Closing channel %lu/%lu",
                        channel->local.id, channel->remote.id);
 
-        if (channel->close_cb) {
-            LIBSSH2_CHANNEL_CLOSE(session, channel);
-        }
         channel->local.close = 1;
 
         channel->close_packet[0] = SSH_MSG_CHANNEL_CLOSE;
@@ -2329,18 +2326,23 @@ channel_close(LIBSSH2_CHANNEL * channel)
 
     if (channel->close_state == libssh2_NB_state_sent) {
         /* We must wait for the remote SSH_MSG_CHANNEL_CLOSE message */
-        if (!channel->remote.close) {
-            libssh2pack_t ret;
 
-            do {
+        while (!channel->remote.close && !rc) {
-                ret = _libssh2_transport_read(session);
+            rc = _libssh2_transport_read(session);
-                if (ret == PACKET_EAGAIN) {
+            if (rc == PACKET_EAGAIN) {
                 return PACKET_EAGAIN;
-                } else if (ret < 0) {
+            }
+            else if (rc < 0)
                 rc = -1;
+            else
+                rc = 0;
         }
-            } while ((ret != SSH_MSG_CHANNEL_CLOSE) && (rc == 0));
     }
+
+    /* We call the callback last in this function to make it keep the local
+       data as long as EAGAIN is returned. */
+    if (channel->close_cb) {
+        LIBSSH2_CHANNEL_CLOSE(session, channel);
     }
 
     channel->close_state = libssh2_NB_state_idle;
@@ -2357,9 +2359,7 @@ LIBSSH2_API int
 libssh2_channel_close(LIBSSH2_CHANNEL *channel)
 {
     int rc;
-
     BLOCK_ADJUST(rc, channel->session, channel_close(channel) );
-
     return rc;
 }
 
@@ -2420,14 +2420,14 @@ libssh2_channel_wait_closed(LIBSSH2_CHANNEL *channel)
 }
 
 /*
- * channel_free
+ * _libssh2_channel_free
  *
  * Make sure a channel is closed, then remove the channel from the session
  * and free its resource(s)
  *
  * Returns 0 on success, negative on failure
  */
-static int channel_free(LIBSSH2_CHANNEL *channel)
+int _libssh2_channel_free(LIBSSH2_CHANNEL *channel)
 {
     LIBSSH2_SESSION *session = channel->session;
     unsigned char channel_id[4];
@@ -2527,7 +2527,7 @@ LIBSSH2_API int
 libssh2_channel_free(LIBSSH2_CHANNEL *channel)
 {
     int rc;
-    BLOCK_ADJUST(rc, channel->session, channel_free(channel));
+    BLOCK_ADJUST(rc, channel->session, _libssh2_channel_free(channel));
     return rc;
 }
 /*

src/channel.h

@@ -60,5 +60,15 @@ int _libssh2_channel_receive_window_adjust(LIBSSH2_CHANNEL * channel,
  */
 int _libssh2_channel_flush(LIBSSH2_CHANNEL *channel, int streamid);
 
+/*
+ * _libssh2_channel_free
+ *
+ * Make sure a channel is closed, then remove the channel from the session
+ * and free its resource(s)
+ *
+ * Returns 0 on success, negative on failure
+ */
+int _libssh2_channel_free(LIBSSH2_CHANNEL *channel);
+
 #endif /* __LIBSSH2_CHANNEL_H */
 

src/session.c

@@ -1,4 +1,4 @@
-/* Copyright (c) 2004-2007, Sara Golemon <sarag@libssh2.org>
+/* Copyright (c) 2004-2007 Sara Golemon <sarag@libssh2.org>
  * Copyright (c) 2009 by Daniel Stenberg
  * All rights reserved.
  *
@@ -1467,7 +1467,8 @@ libssh2_poll(LIBSSH2_POLLFD * fds, unsigned int nfds, long timeout)
         }
 
         if (active_fds) {
-            /* Don't block on the sockets if we have channels/listeners which are ready */
+            /* Don't block on the sockets if we have channels/listeners which
+               are ready */
             timeout_remaining = 0;
         }
 #ifdef HAVE_POLL

src/sftp.c

@@ -38,6 +38,7 @@
 
 #include "libssh2_priv.h"
 #include "libssh2_sftp.h"
+#include "channel.h"
 
 /* Note: Version 6 was documented at the time of writing
  * However it was marked as "DO NOT IMPLEMENT" due to pending changes
@@ -83,6 +84,8 @@
 /* S_IFDIR */
 #define LIBSSH2_SFTP_ATTR_PFILETYPE_DIR         0040000
 
+static int sftp_close_handle(LIBSSH2_SFTP_HANDLE *handle);
+
 /* libssh2_htonu64
  */
 static void
@@ -516,10 +519,19 @@ LIBSSH2_CHANNEL_CLOSE_FUNC(libssh2_sftp_dtor)
     (void) session_abstract;
     (void) channel;
 
+#if 0
+    /* EEEK! While it might sound like a neat idea to make this code loop over
+       all the outstanding handles and close them, that is going to cause
+       EAGAIN to get returned and this callback system is not designed to
+       handle this very nicely so thus we now DEMAND that the app closes its
+       handles instead!
+    */
+
     /* Loop through handles closing them */
     while (sftp->handles) {
-        libssh2_sftp_close_handle(sftp->handles);
+        sftp_close_handle(sftp->handles);
     }
+#endif
 
     /* Free the partial packet storage for sftp_packet_read */
     if (sftp->partial_packet) {
@@ -688,7 +700,8 @@ static LIBSSH2_SFTP *sftp_init(LIBSSH2_SESSION *session)
     }
     LIBSSH2_FREE(session, data);
 
-    /* Make sure that when the channel gets closed, the SFTP service is shut down too */
+    /* Make sure that when the channel gets closed, the SFTP service is shut
+       down too */
     session->sftpInit_sftp->channel->abstract = session->sftpInit_sftp;
     session->sftpInit_sftp->channel->close_cb = libssh2_sftp_dtor;
 
@@ -718,61 +731,79 @@ LIBSSH2_API LIBSSH2_SFTP *libssh2_sftp_init(LIBSSH2_SESSION *session)
     return ptr;
 }
 
+/*
+ * sftp_shutdown
+ *
+ * Shutsdown the SFTP subsystem
+ */
+static int
+sftp_shutdown(LIBSSH2_SFTP *sftp)
+{
+    int rc;
+    LIBSSH2_SESSION *session = sftp->channel->session;
+    /*
+     * Make sure all memory used in the state variables are free
+     */
+    if (sftp->partial_packet) {
+        LIBSSH2_FREE(session, sftp->partial_packet);
+        sftp->partial_packet = NULL;
+    }
+    if (sftp->open_packet) {
+        LIBSSH2_FREE(session, sftp->open_packet);
+        sftp->open_packet = NULL;
+    }
+    if (sftp->readdir_packet) {
+        LIBSSH2_FREE(session, sftp->readdir_packet);
+        sftp->readdir_packet = NULL;
+    }
+    if (sftp->write_packet) {
+        LIBSSH2_FREE(session, sftp->write_packet);
+        sftp->write_packet = NULL;
+    }
+    if (sftp->fstat_packet) {
+        LIBSSH2_FREE(session, sftp->fstat_packet);
+        sftp->fstat_packet = NULL;
+    }
+    if (sftp->unlink_packet) {
+        LIBSSH2_FREE(session, sftp->unlink_packet);
+        sftp->unlink_packet = NULL;
+    }
+    if (sftp->rename_packet) {
+        LIBSSH2_FREE(session, sftp->rename_packet);
+        sftp->rename_packet = NULL;
+    }
+    if (sftp->mkdir_packet) {
+        LIBSSH2_FREE(session, sftp->mkdir_packet);
+        sftp->mkdir_packet = NULL;
+    }
+    if (sftp->rmdir_packet) {
+        LIBSSH2_FREE(session, sftp->rmdir_packet);
+        sftp->rmdir_packet = NULL;
+    }
+    if (sftp->stat_packet) {
+        LIBSSH2_FREE(session, sftp->stat_packet);
+        sftp->stat_packet = NULL;
+    }
+    if (sftp->symlink_packet) {
+        LIBSSH2_FREE(session, sftp->symlink_packet);
+        sftp->symlink_packet = NULL;
+    }
+
+    rc = _libssh2_channel_free(sftp->channel);
+
+    return rc;
+}
+
 /* libssh2_sftp_shutdown
  * Shutsdown the SFTP subsystem
  */
 LIBSSH2_API int
 libssh2_sftp_shutdown(LIBSSH2_SFTP *sftp)
 {
-    /*
+    int rc;
-     * Make sure all memory used in the state variables are free
+    BLOCK_ADJUST(rc, sftp->channel->session,
-     */
+                 sftp_shutdown(sftp));
-    if (sftp->partial_packet) {
+    return rc;
-        LIBSSH2_FREE(sftp->channel->session, sftp->partial_packet);
-        sftp->partial_packet = NULL;
-    }
-    if (sftp->open_packet) {
-        LIBSSH2_FREE(sftp->channel->session, sftp->open_packet);
-        sftp->open_packet = NULL;
-    }
-    if (sftp->readdir_packet) {
-        LIBSSH2_FREE(sftp->channel->session, sftp->readdir_packet);
-        sftp->readdir_packet = NULL;
-    }
-    if (sftp->write_packet) {
-        LIBSSH2_FREE(sftp->channel->session, sftp->write_packet);
-        sftp->write_packet = NULL;
-    }
-    if (sftp->fstat_packet) {
-        LIBSSH2_FREE(sftp->channel->session, sftp->fstat_packet);
-        sftp->fstat_packet = NULL;
-    }
-    if (sftp->unlink_packet) {
-        LIBSSH2_FREE(sftp->channel->session, sftp->unlink_packet);
-        sftp->unlink_packet = NULL;
-    }
-    if (sftp->rename_packet) {
-        LIBSSH2_FREE(sftp->channel->session, sftp->rename_packet);
-        sftp->rename_packet = NULL;
-    }
-    if (sftp->mkdir_packet) {
-        LIBSSH2_FREE(sftp->channel->session, sftp->mkdir_packet);
-        sftp->mkdir_packet = NULL;
-    }
-    if (sftp->rmdir_packet) {
-        LIBSSH2_FREE(sftp->channel->session, sftp->rmdir_packet);
-        sftp->rmdir_packet = NULL;
-    }
-    if (sftp->stat_packet) {
-        LIBSSH2_FREE(sftp->channel->session, sftp->stat_packet);
-        sftp->stat_packet = NULL;
-    }
-    if (sftp->symlink_packet) {
-        LIBSSH2_FREE(sftp->channel->session, sftp->symlink_packet);
-        sftp->symlink_packet = NULL;
-    }
-
-    return libssh2_channel_free(sftp->channel);
 }
 
 /* *******************************