On Tue, 23 Sep 2014 18:18:24 +0200 Karsten Otto <[email protected]> wrote:
> Am 22.09.2014 um 10:49 schrieb Pekka Paalanen <[email protected]>: > > > On Thu, 11 Sep 2014 21:42:26 +0200 > > Karsten Otto <[email protected]> wrote: > > > >> From: Philip Withnall <[email protected]> > >> Date: Fri, 15 Feb 2013 12:57:05 +0000 > >> > >> This happens if the socket has been gracefully closed. > >> > >> Signed-off-by: Philip Withnall <[email protected]> > >> Signed-off-by: Karsten Otto <[email protected]> > >> --- > >> src/wayland-server.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/src/wayland-server.c b/src/wayland-server.c > >> index 674aeca..83e6f83 100644 > >> --- a/src/wayland-server.c > >> +++ b/src/wayland-server.c > >> @@ -260,7 +260,7 @@ wl_client_connection_data(int fd, uint32_t mask, void > >> *data) > >> len = 0; > >> if (mask & WL_EVENT_READABLE) { > >> len = wl_connection_read(connection); > >> - if (len < 0 && errno != EAGAIN) { > >> + if (len <= 0 && errno != EAGAIN) { > >> wl_client_destroy(client); > >> return 1; > >> } > > > > I don't think this is correct. If wl_connection_read() returns zero, it > > is because recvmsg() returned zero, which means errno has not been set. > > If some earlier call caused errno to become EAGAIN, you still won't hit > > the destroy path. > > > Right, better make the EOF case explicit and change the line to > > if (len == 0 || (len < 0 && errno != EAGAIN)) > > > Why do we need this change here anyway? That would be good to explain > > in the commit message. Does it fix a bug that happens only on BSD? > > Or is it just avoid one more spin through the poll loop? > > > This change is necessary when using something other than epoll for event I/O > handling. > > Currently execution should never reach the point where recvmsg returns EOF > (len == 0). Instead, epoll will detect this and indicate EPOLLHUP, which is > handeled a few lines above, closing the connection. However, other event > mechanisms may not be able to distinguish EOF and regular readability (e.g. > select), or at least not consistently across platforms (e.g. POLLHUP). There > is also the potential of half-closed connections (shutdown / POLLRDHUP), > though I am not sure this is an issue with wayland. > > In any case, I believe it is safer to catch the EOF result explicitly here. > Without it, I got a nasty infinite loop (OSX with libev select backend). > Ah, excellent explanation. This should definitely be in the commit message. :-) > > There is also a hypothetical problem here. If the fd polls readable, > > but there isn't any data to be read, you would disconnect the client. > > Arguably that should never happen, but should we trust that? The read > > is explicitly non-blocking as well. > > > If there is nothing to read from a nonblocking socket, recvmsg and friends > should return -1 and set errno to EAGAIN. Unless they are not POSIX > conformant, in which case you are in deep waters anyway. There is also the > pathological case of a zero-size buffer, but that should be prevented > elsewhere. > Oh yeah, I suppose. Ok. Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
