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

[Aris Adamantiadis <aris@xxxxxxxxxxxx>]

Hi Nikolay,

No problem, as I understand you're using revents as a kind of a flag to
detect when the output buffer is totally empty. It's not exactly how it
was designed :)

I'm not sure I fully understand the sequence of actions here. Are we
somewhere in (backtrace)
ssh_event_dopoll()
____ssh_socket_event_callback()
________ssh_channel_select()

Are the parameters to these functions SSH sessions? Because if so you
may enter in reentrancy zone and you may have unexpected results.
The best way to handle the spurious POLLOUT is to add the current ssh
session into an ssh_event
LIBSSH_API int ssh_event_add_session(ssh_event event, ssh_session session);
and then call ssh_event_dopoll() with timeout=0

I'm not sure how it would fit into your architecture though. Maybe you
should use the channel callbacks instead of ssh_channel_select() inside
of global socket callbacks.

Hope it helps,

Aris


On 2/08/17 16:36, Nikolay wrote:
> Hi Aris,
>
> Sorry, may be I'm wrong with this patch and didn't quite understand
> the purpose of this function.
>
> But I'm currently encountering a problem when working with libssh in
> async non-blocking mode.
> I have multiple ssh connections and I do polling in the following way:
>
> while (1) {
>   // adding ssh session sockets to eventloop
>   // with events taken from ssh_get_poll_flags()
>   // also operating with my other sockets here
>   ssh_event_dopoll(eventloop, timeout)
>   // removing ssh session sockets from eventloop
> }
>
> In ssh socket event callback:
> if (revents != 0) {
>   if (!session_connected_and_authorized) {
>     call_ssh_connect_or_auth_again();
>   } else {
>     ssh_channel_select(session channels for read, write and except);
>     // open/exec/close on channels of this session
>   }
> }
>
> The problem is that when I close the last channel in session,
> ssh_channel_close() sends a packet with ssh_socket_unbuffered_write()
> and enabled POLLOUT on this session socket.
> Then my program gets stuck in socket event callback getting POLLOUT
> over and over again.
> Because the session have no channels now, ssh_channel_exec() doesn't
> do anything with session socket and it keeps receiving POLLOUT event.
> May be you know any hint how to avoid this? Except creating a dummy
> channel :-)
> Or may be I'm doing it all wrong?
>
> Thank you in advance!
>
>
> Best regards, Nikolay Karikh.
>
>
>> Hi Nikolay,
>>
>> Thanks for your patch. Is there a particular reason it had to be
>> changed? The current behavior of the POLLOUT is to avoid a poll() call
>> when sending a new packet. I think the new behavior will force buffering
>> + return to the main loop + poll() + send() instead of send() + main
>> loop + poll() + main loop again. It can look nasty in debug logs but I
>> think it's better for latency.
>>
>> Aris
>>
>
>