Subject: Re: Callback for channel data ready

Re: Callback for channel data ready

From: Tom Weber <tom.weber_at_cryptzone.com>
Date: Fri, 2 Mar 2012 12:21:18 +0100

Hi!

These are my thoughts...

> On Wed, Feb 29, 2012 at 7:42 PM, Peter Stuge <peter_at_stuge.se> wrote:
>> > > Sorry not at this time. I think we want callbacks though, so feel
>> > > free to discuss and send patches. :)

Callbacks sound nice in theory, but I've had my share of them, wrestling with
PuTTY and Symbian code, and particularly those two together, which have mostly
incompatible callback schemes.

The problem is that you as an application developer don't control the context
in which the callback is called.

1) You might want to disconnect and free the session, but you can't do that
since you have to return to a libssh2_session_ routine, which expects the
session to be valid.

2) You might want to process the event later, and then you need an additional
mechanism to save the event.

3) During which function call, and from which thread, can we count on the
callback being called? We must know that exactly, to handle things like
mutexes and resource ownership correctly. (Preferably the callback pointer
should be provided at the function call and not saved, to make this more
clear.)

By the way, signals suffer from the same problems...

>> There is no such thing.. libssh2 has evolved, and must continue to
>> evolve. The libssh2 API can and should be expanded to become more
>> useful, and the definition of useful should basically come from it's
>> users.

Agreed! :-)

On Fri, March 2, 2012 03:26, Mitchell Hashimoto wrote:
> Instead, I believe it would be nice to do something like the following:
>
> int libssh2_session_ready_channels(LIBSSH2_SESSION *, LIBSSH2_CHANNEL **)
>
> Where this would return a LIBSSH2_ERROR_EAGAIN if the socket would block
> when nonblocking is on, and the return value would basically behave just
> like any other libssh2 call. It would return `0` when it was a success.
> Normal libssh2 behavior. If 0 is returned, then the second parameter is set
> to a pointer to a single channel that is ready to be read, or NULL if none
> are ready.

I suppose that libssh2 maintains a queue of packets for various channels, and
your proposed function would return the first item from the queue? Sounds good
if you want to process all packets in order, but what if you want to skip some
channels because their corresponding sockets are not ready to accept more
data, and you want libssh2 to save the data for later, while blocking that
channel?

You might think, "why not save such data in a buffer instead?", but that's not
the same thing. If you keep reading data which you can't send, libssh2 will
adjust the window and accept more and more, and then you might run out of
memory trying to buffer it all.

> Again, like I said, I don't know the internal structure of libssh2 or
> the feasibility of such a thing, but I can see looping through channels to
> become overwhelming.

Overwhelming? Well, the loop must be done in some way anyway inside the
library. (I suppose that libssh2 maps from a channel ID to a struct pointer,
and that might be a hash array lookup?)

Again, see my point above about skipping channels which must be paused. One
way would be to tell libssh2 that a particular channel shouldn't be checked.
Another way would be to loop and check selectively.

I'm quite happy with libssh2 as it is now, but I'd also like a nicer way to
check a channel. Right now I have a helper routine which looks like this:

static long _try_channel_read(... LIBSSH2_CHANNEL *channel)
{
    long read_size;

    /* ugly kludge */
    libssh2_session_set_blocking(session->ssh_session, 0);

    read_size = libssh2_channel_read(channel,
                                     session->io_buffer,
                                     session->io_buffer_capacity);
    ...

    /* ugly kludge */
    libssh2_session_set_blocking(session->ssh_session, 1);

    return read_size;
}

-- 
Best regards,
Tom Weber
Cryptzone Group AB
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2012-03-02