BUG/MINOR: raw_sock: always perfom the last recv if RDHUP is not available
Curu Wong reported a case where haproxy used to send RST to a server
after receiving its FIN. The problem is caused by the fact that being
a server connection, its fd is marked with linger_risk=1, and that the
poller didn't report POLLRDHUP, making haproxy unaware of a pending
shutdown that came after the data, so it used to resort to nolinger
for closing.
However when pollers support RDHUP we're pretty certain whether or not
a shutdown comes after the data and we don't need to perform that extra
recv() call.
Similarly when we're dealing with an inbound connection we don't care
and don't want to perform this extra recv after a request for a very
unlikely case, as in any case we'll have to deal with the client-facing
TIME_WAIT socket.
So this patch ensures that only when it's known that there's no risk with
lingering data, as well as in cases where it's known that the poller would
have detected a pending RDHUP, we perform the fd_done_recv() otherwise we
continue, trying a subsequent recv() to try to detect a pending shutdown.
This effectively results in an extra recv() call for keep-alive sockets
connected to a server when POLLRDHUP isn't known to be supported, but
it's the only way to know whether they're still alive or closed.
This fix should be backported to 1.7, 1.6 and 1.5. It relies on the
previous patch bringing support for the HAP_POLL_F_RDHUP flag.
diff --git a/src/raw_sock.c b/src/raw_sock.c
index f24f201..8aba58d 100644
--- a/src/raw_sock.c
+++ b/src/raw_sock.c
@@ -296,13 +296,21 @@
if (ret < try) {
/* unfortunately, on level-triggered events, POLL_HUP
* is generally delivered AFTER the system buffer is
- * empty, so this one might never match.
+ * empty, unless the poller supports POLL_RDHUP. If
+ * we know this is the case, we don't try to read more
+ * as we know there's no more available. Similarly, if
+ * there's no problem with lingering we don't even try
+ * to read an unlikely close from the client since we'll
+ * close first anyway.
*/
if (fdtab[conn->t.sock.fd].ev & FD_POLL_HUP)
goto read0;
- fd_done_recv(conn->t.sock.fd);
- break;
+ if ((!fdtab[conn->t.sock.fd].linger_risk) ||
+ (cur_poller.flags & HAP_POLL_F_RDHUP)) {
+ fd_done_recv(conn->t.sock.fd);
+ break;
+ }
}
count -= ret;
}