On Wed, 2019-05-29 at 16:02 +0200, Ondrej Zajicek wrote: > On Mon, May 27, 2019 at 03:29:26PM +0000, Kenth Eriksson wrote: > > On Mon, 2019-05-27 at 17:12 +0200, Ondrej Zajicek wrote: > > > CAUTION: This email originated from outside of the organization. > > > Do > > > not click links or open attachments unless you recognize the > > > sender > > > and know the content is safe. > > > > > > > > > On Mon, May 27, 2019 at 02:28:52PM +0200, Kenth Eriksson wrote: > > > > Datagram sockets may return 0 and stream sockets can return 0 > > > > if the requested number of bytes to read is 0. > > > > > > Hi > > > > > > You mean that if count arg to read() is 0? > > > > > > How that may happen? > > > > > We have a client remote controlling bird using a socket that did > > get > > get POLLHUP, maybe due to that bird closed the socket. > > > > Don't think checking for 0 is enough, from man page; > > Hi > > Did you examine the possible causes of why BIRD closed the socket? > That could be done by adding log messages to sk_read(), perhaps > limited > to cases when s->type == SK_UNIX. The man page describes two > additional > cases, so it is a good idea to try to distinguish which one happened. > We are still trying to reproduce.
> Also, i don't think that check for POLLHUP is correct, as that flag > generally means socket is closed for write, not that it is closed for > read (although some OSes are less consistent in this matter than > others). > POLLOUT and POLLHUP are mutually exclusive. But POLLIN and POLLHUP are NOT mutually exclusive. But I can't see that the socket will transition from POLLHUP|POLLIN to POLLIN only. As I see it there are three ways to tell if a socket is closed; 1) write returns EPIPE 2) read returns 0 when requesting to read n bytes where n > 0 3) POLLHUP from poll (irrespective if read or write) > This is particularly fragile part of code, depends on OS API details, > so > i would avoid to do changes there unless we are 100% sure that they > are > proper fix for some confirmed condition. > Unserstood. If we are able to reproduce, we should be able to confirm or update patch. > > > "When a stream socket peer has performed an orderly shutdown, the > > return value will be 0 (the traditional "end-of-file" return). > > > > Datagram sockets in various domains (e.g., the UNIX and Internet > > domains) permit zero-length datagrams. When such a datagram is > > received, the return value is 0. > > Well, we open the socket as SOCK_STREAM and not SOCK_DGRAM, so i > would > expect this case does not apply. (Although i have no idea what would > happen > if the other side opens the socket with SOCK_DGRAM.) > > And even if this case would happen, we should ignore it and not fall > back > to call_rx_hook() path. > > > > The value 0 may also be returned if the requested number of bytes > > to > > receive from a stream socket was 0." > > This case means that we called read() with (s->rpos == s->rbuf + s- > >rbsize), > buffer is full and last call_rx_hook() does not eat data from it. > That > means something is wrong, e.g. an incomplete message filled the > entire > buffer and without resizing buffer we would just end in endless loop, > therefore closing the connection with error is safest thing to do. >
