Dan Fandrich <[EMAIL PROTECTED]> writes:
> On Thu, Apr 24, 2008 at 08:35:34PM -0700, J.T. Conklin wrote:
>> libssh2_sftp_close_handle() currently returns -1 on failure. What is
>> client code supposed to do to handle this?
>
> Good question. If the failure is due to an out of memory condition, then
> trying again when more memory is available is probably the right thing.
> But in other cases, like if the socket is already closed, then there's
> nothing you can do. Problem is, some resources aren't freed in the latter
> case, putting you into a catch-22 situation. That should really be fixed
> (somehow) in the library.
Sorry all, it has taken a while for this to reach the top of my queue
again.
As I said in my original message, having libssh2_sftp_close_handle()
return errors on failures due to a closed socket, etc. results in an
infinate loop in libssh2_sftp_dtor.
For our product, I changed libssh2_sftp_close_handle() to do a "best
effort" close, which ensures all resources are closed and cleaned up
(to the greatest extent possible). This is similar to other protocol
libraries, eg. OpenLDAP's ldap_unbind() function. We've been using
this change for the last few months without issues (in the same
hostile network environment that used to regularly trigger the
failure).
I checked the current CVS version of sftp.c, and it looks like more
effort has been made to make libssh2_sftp_close_handle() return
PACKET_EAGAIN when an underlying libssh_* operation fails with
PACKET_EAGAIN. I'm not sure what benefit this is supposed to provide.
It seems to push error handling for nonblocking connections up to the
client, so what was a "libssh2_sftp_close_handle(h);" has to be
changed to "while(libssh2_sftp_close_handle(h) != PACKET_EAGAIN);".
I really think this libssh2_sftp_close_handle() needs to be changed
to be best effort and blocking, like ldap_unbind(). Otherwise sftp
in practice can not be used reliabily. After looking at the bugs
filed @ SF, I think this is the same issue as reported as 1940276.
--jtc
FWIW, this is the patch we're using against a CVS snapshot taken on
2007/05/16.
Index: src/sftp.c
===================================================================
--- src/sftp.c (revision 11094)
+++ src/sftp.c (working copy)
@@ -1502,13 +1502,15 @@
unsigned long data_len, retcode, request_id;
ssize_t packet_len = handle->handle_len + 13; /* packet_len(4) +
packet_type(1) + request_id(4) + handle_len(4) */
unsigned char *packet, *s, *data;
- int rc;
+ int rc;
+ int retval = 0;
_libssh2_debug(session, LIBSSH2_DBG_SFTP, "Closing handle");
s = packet = LIBSSH2_ALLOC(session, packet_len);
if (!packet) {
libssh2_error(session, LIBSSH2_ERROR_ALLOC, "Unable to
allocate memory for FXP_CLOSE packet", 0);
- return -1;
+ retval = -1;
+ goto error;
}
libssh2_htonu32(s, packet_len - 4);
s += 4;
@@ -1521,7 +1523,8 @@
if (packet_len != libssh2_channel_write(channel, (char *)packet,
packet_len)) {
libssh2_error(session, LIBSSH2_ERROR_SOCKET_SEND, "Unable to
send FXP_CLOSE command", 0);
LIBSSH2_FREE(session, packet);
- return -1;
+ retval = -1;
+ goto error;
}
LIBSSH2_FREE(session, packet);
@@ -1531,7 +1534,8 @@
}
if (rc) {
libssh2_error(session, LIBSSH2_ERROR_SOCKET_TIMEOUT, "Timeout
waiting for status message", 0);
- return -1;
+ retval = -1;
+ goto error;
}
retcode = libssh2_ntohu32(data + 5);
@@ -1540,9 +1544,10 @@
if (retcode != LIBSSH2_FX_OK) {
sftp->last_errno = retcode;
libssh2_error(session, LIBSSH2_ERROR_SFTP_PROTOCOL, "SFTP
Protocol Error", 0);
- return -1;
+ retval = -1;
}
+error:
if (handle == sftp->handles) {
sftp->handles = handle->next;
}
@@ -1558,7 +1563,7 @@
LIBSSH2_FREE(session, handle->handle);
LIBSSH2_FREE(session, handle);
- return 0;
+ return retval;
}
/* }}} */
--
J.T. Conklin
-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
libssh2-devel mailing list
libssh2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libssh2-devel
