Re: [PATCH] socket: do not enable POLLOUT for empty out buffer

[Nikolay <nitro@xxxxxxxxxxx>]

> The idea is not that we "wait", but we pass the flag to the kind of
> events we're interested in. Usually on the next main loop iteration,
> poll returns with POLLOUT and we can set write_wont_block=1. We do this
> in order to, in future, be able to send() the next packet without having
> to wait for a main loop iteration. In the end we do the same amount of
> poll operations. We cannot send() anything without being sure the write
> operation will not block.
= I think *if your socket is in non-blocking mode*, you can call send() 
and be sure it wouldn't block. If the internal buffer is full, send() 
will just return 0 meaning it can't accept any more data right now. So 
if send returns something less than your data chunk size, you need to 
postpone sending the rest of the chunk until you get POLLOUT - and this 
is the case for it.

> We probably need better ways of querying the ssh session to know in what
> state it is (connecting/authenticating/disconnecting/errored)
= I agree. For now I had to do it myself in my application.

> Actually from inside certain callbacks, some ssh functions will not
> work, like functions that expect reading something additional from the
> network. A good example is ssh_channel_open() in blocking mode.
= I have made several stress tests and everything seems fine for now 
(but again, I'm using non-blocking mode only).

Although I have run into some problems, such as:

1) ssh_channel_select() not always returning the channel (usually in 
chansexcept) when it finally becomes opened. My current workaround is to 
always call ssh_channel_open() on unopened (my internal state) channels 
when session socket have POLLIN.

2) confusing result of ssh_channel_is_closed(). It may return 1 (after 
both sides sent EOF), but you still have to call ssh_channel_close(), 
otherwise the server will hold that channel and would not let you open 
more than 10 (default MaxSessions) channels in total.

3) This POLLOUT issue when all channels are closed. Current workaround 
is a dummy channel.

> Normally the API is ok to use in non blocking mode, actually it makes no
> sense to use blocking/synchronous APIs in an asynchronous design.
> I just looked at the code again and I have no idea why it even mentions
> blocking mode. I think it's because every synchronous API calls you will
> make on a session you have added to an event handle will impact the
> other sessions in that same event handle, because if libssh has to do a
> synchronous recv() on SSH session 1, it has to run ssh_event_dopoll() on
> the event structure that contains ssh sessions 1 and 2.
> If your code is properly using callbacks it should work.
> > I think it's the reason I failed with using ssh_event_add_session()
> > and session callbacks. I did get some SIGSEGV during those experiments.
> Maybe reentrancy issues? Send me a few backtraces, they can help.
= That would be great. May be I missed something, because this is what I 
have started from (ssh_event_add_session+callbacks), rookie mistakes are 
always possible.

I would make some rewritings and tests and then come back with the 
results. Thank you!


Best regards, Nikolay Karikh.

Attachment: smime.p7s
Description: Криптографическая подпись S/MIME