Ticket #54 (closed defect)

Opened 9 years ago

Last modified 8 years ago

libssh2-0.18 session.c

Reported by: gmiller1018 Owned by: bagder
Priority: normal Milestone:
Component: API Version:
Keywords: Cc: gmiller1018, bagder, afullfor
Blocked By: Blocks:

Description

I have noticed an issue with libssh2_poll on my Linux machines. I found that on my machines HAVE_POLL and HAVE_SELECT are defined and the code that is executed in the poll code not the select. I fixed this by just undef'ing the HAVE_POLL.

The other issue here is when the select code is enabled the code in the libssh2_poll seems to be in error. The code as I fixed it is:
[code]

if (sysret > 0) {

for(i = 0; i < nfds; i++) {

switch (fds[i].type) {
case LIBSSH2_POLLFD_SOCKET:

if (FD_ISSET(fds[i].fd.socket, &rfds)) {

fds[i].revents |= LIBSSH2_POLLFD_POLLIN;

}
if (FD_ISSET(fds[i].fd.socket, &wfds)) {

fds[i].revents |= LIBSSH2_POLLFD_POLLOUT;

}
if (fds[i].revents) {

active_fds++;

}
break;

case LIBSSH2_POLLFD_CHANNEL:

if (FD_ISSET(fds[i].fd.channel->session->socket_fd, &rfds)) {

/* Spin session until no data available */
while (libssh2_packet_read(fds[i].fd.channel->session)
> 0);
libssh2_packet_read(fds[i].fd.channel->session);
active_fds++;

}
break;

case LIBSSH2_POLLFD_LISTENER:

if (FD_ISSET

(fds[i].fd.listener->session->socket_fd, &rfds)) {
/* Spin session until no data available */
while (libssh2_packet_read(fds[i].fd.listener->session)
> 0);
libssh2_packet_read(fds[i].fd.listener->session);
active_fds++;

}
break;

}

}

}

code

when the FD_ISSET is true the code was looping reading (libssh2_packet_read) till and error occured but the behavio was reading till blocked. I changed the code to just read once (since that is all the FD_ISSET guaranteed). The increment of active_fds was also missing so the loop would not exit when it needs to.

There are still some cases where I think there is data but it is not being returned for some reason. I have not been able to identify the cause but I am working on it.

In my use of the poll function I am monitiring sockets of my own and sockets under control of libssh2 so my use may not be typical.

should there be some reason that my changes are breaking something and there is another way to fix this please let me know.

Change History

comment:1 Changed 9 years ago by gmiller1018

It appears from my experimentation that the looping to read with the libssh2_packet_read is needed so the ONLY way this code area will work is if the sockets are made NON BLOCKING and the increment of the active_fds is there. If this truly is a requirement then I would like to see the documentation changed or the code check to see of the socket is in non blocking mode and do something to indicate that this is a problem.

comment:2 Changed 9 years ago by bagder

Well of course it is a bug if it only works in non-blocking mode, as it is supposed to always work. Personally I only use libssh2 non-blocking... So just changing the docs isn't the proper fix.

The socket could of course be made non-blocking just for this loop... Perhaps that is the best fix at least short-term?

comment:3 Changed 9 years ago by afullfor

Similar problems on a FreeBSD system. The libssh2_packet_read() approach only works if the channel is set to non-blocking. Also, the select() call on line 1481 of session.c is passing "maxfd" as the first argument. The correct value is the number of fds to check, which is actually "maxfd+1". I suspect this is the cause of the additonal issues Gary is seeing as the select() will not report activity on the numerically highest fd,

comment:4 Changed 9 years ago by bagder

I've committed a fix that uses maxfd+1

comment:5 Changed 8 years ago by sf-robot

This Tracker item was closed automatically by the system. It was
previously set to a Pending status, and the original submitter
did not respond within 14 days (the time period specified by
the administrator of this Tracker).

Note: See TracTickets for help on using tickets.