Subject: [libssh2] Proposal for Non-blocking changes

[libssh2] Proposal for Non-blocking changes

From: James Housley <jim_at_thehousleys.net>
Date: Fri, 27 Apr 2007 05:47:21 -0400

I am proposing and planning to start working on the following
changes, after discussion and agreement from the other readers of
this list.

BACKGROUND:

The current implementation, 0.14 and 0.15-CVS, does not properly
handle a socket in non-blocking mode. Work was started by Daniel
Stenberg and then continued by myself. But this direction was based
on adding a new non-blocking function for every existing function.

Libssh2 has a hierarchical setup, http://www.libssh2.org/wiki/
index.php/SSH2:Overview . You start with LIBSSH2_SESSION, that owns
the socket. Then to LIBSSH2_SESSION you attach a LIBSSH2_CHANNEL to
transfer data. Then on the LILBSSH2_CHANNEL you can attach
LIBSSH2_SFTP or other protocols. It is also possible to have more
then one LIBSSH2_CHANNEL attached to a single LIBSSH2_SESSION. The
current functions to control blocking would change blocking in the
channel, and change the setting on the socket (LIBSSH2_SESSION).
This is clearly wrong. The blocking functions belong at the
LIBSSH2_SESSION level, later I will talk about advanced possibilities.

PROPOSAL:

I propose making libssh2 fully support non-blocking mode all the way
down to the LIBSSH2_SESSION level.

1) Move the blocking functions into session.c, and modify the ones in
channel.c to call the ones in session.c.

2) Create _libssh2_get_socket_blocking() that will be called by
libssh2_session_startup() to try and find the current blocking state
of the socket, defaulting to blocking when not sure.

3) The start working from session.c and find all routines that call
send(2) and recv(2) and have them return EAGAIN when send(2) or recv
(2) returns EAGAIN. But, since the calling routine won't properly
support EAGAIN, I will loop on EAGAIN so the code will still work and
act as if it were in blocking mode.

Currently in session.c, libssh2_banner_send() and
libssh2_banner_receive() aren't non-blocking safe. In transport.c,
send_existing(), libssh2_packet_write() and libssh2_packet_read() are
non-blocking safe, but all callers may not be.

For example, from libssh2_session_startup()

if (libssh2_banner_send(session)) {
        /* Unable to send banner? */
        libssh2_error(session, LIBSSH2_ERROR_BANNER_SEND,
                      "Error sending banner to remote host", 0);
        return LIBSSH2_ERROR_BANNER_SEND;
}

would become

while ((rc = libssh2_banner_send(session)) == EAGAIN) {
     libssh2_waitsocket(session, LIBSSH2_READ_TIMEOUT);
}
if (rc) {
        /* Unable to send banner? */
        libssh2_error(session, LIBSSH2_ERROR_BANNER_SEND,
                      "Error sending banner to remote host", 0);
        return LIBSSH2_ERROR_BANNER_SEND;
}

If the socket wasn't in non-blocking mode, EAGAIN is not a valid
return value so the while wouldn't loop.

There are also many places that libssh2_channel_get_blocking()/
libssh2_channel_set_blocking() pairs were added to the code. But,
since all channels share a single LIBSSH2_SESSION this is wrong.
These need to be removed and temporarily replaced with "while ((rc =
libssh2_*(session)) == EAGAIN){libssh2_waitsocket(session,
LIBSSH2_READ_TIMEOUT);}" loops.

4) After all the lowest level routines can return EAGAIN, move up to
the next set of routines and continue until there are no more
functions that have "while ((rc = libssh2_*(session)) == EAGAIN)
{libssh2_waitsocket(session, LIBSSH2_READ_TIMEOUT);}" code.

There are some external functions, and probably some internal ones
too, that don't return a numerical value, but return a structure.
For example, libssh2_sftp_open_ex() returns a LIBSSH2_SFTP_HANDLE. I
think the current functions should operate in a blocking mode and
loop on EAGAIN when necessary and create a second non-blocking
version. This will keep the API consistent and only add a few new
functions. The list is small, not including the defined short cut
functions of these.

LIBSSH2_API LIBSSH2_SESSION *libssh2_session_init_ex()
LIBSSH2_API LIBSSH2_CHANNEL *libssh2_channel_open_ex();
LIBSSH2_API LIBSSH2_CHANNEL *libssh2_channel_direct_tcpip_ex();
LIBSSH2_API LIBSSH2_LISTENER *libssh2_channel_forward_listen_ex();
LIBSSH2_API LIBSSH2_CHANNEL *libssh2_channel_forward_accept();
LIBSSH2_API LIBSSH2_CHANNEL *libssh2_scp_recv();
LIBSSH2_API LIBSSH2_CHANNEL *libssh2_scp_send_ex();
LIBSSH2_API LIBSSH2_PUBLICKEY *libssh2_publickey_init();
LIBSSH2_API LIBSSH2_SFTP *libssh2_sftp_init();
LIBSSH2_API LIBSSH2_SFTP_HANDLE *libssh2_sftp_open_ex();

ADVANCED:

After the above work is all done it would be possible to add code,
maybe a lot, into the channel functions to allow channels to simulate
blocking by looping when the LIBSSH2_SESSION is non-blocking. I am
not sure why you would want some blocking and others not. But this
could be done, not that I am recommending it.

Comments wanted from all.

Jim

--
/"\   ASCII Ribbon Campaign  .
\ / - NO HTML/RTF in e-mail  .
  X  - NO Word docs in e-mail .
/ \ -----------------------------------------------------------------
jeh@FreeBSD.org      http://www.FreeBSD.org     The Power to Serve
jim@TheHousleys.Net  http://www.TheHousleys.net
---------------------------------------------------------------------
The wise man built his network upon Un*x.
     The foolish man built his network upon Windows.
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
libssh2-devel mailing list
libssh2-devel_at_lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libssh2-devel
Received on 2007-04-27