[BUG] stream_sock: don't stop reading when the poller reports an error

As reported by Jean-Baptiste Quenot and Robbie Aelter, sometimes a
backend server error is converted to a 502 error if the backend stops
before reading all the request. The reason is that the remote system
sends a TCP RST packet because there are still unread data pending in
the socket buffer. This RST is translated as a socket error on the
local system, and this error is reported by the poller.

However, most of the time, it's a write error, but the system is
still able to read the remaining pending data, such as in the trace
below :

send(7, "GET /aaa HTTP/1.0\r\nUser-Agent: Mo"..., 1123, MSG_DONTWAIT|MSG_NOSIGNAL) = 1123
epoll_ctl(3, EPOLL_CTL_ADD, 7, {EPOLLIN, {u32=7, u64=7}}) = 0
epoll_wait(3, {{EPOLLIN|EPOLLERR|EPOLLHUP, {u32=7, u64=7}}}, 8, 1000) = 1
gettimeofday({1247593958, 643572}, NULL) = 0
recv(7, "HTTP/1.0 400 Bad request\r\nCache-C"..., 7000, MSG_NOSIGNAL) = 187
setsockopt(6, SOL_TCP, TCP_NODELAY, [0], 4) = 0
setsockopt(6, SOL_TCP, TCP_CORK, [1], 4) = 0
send(6, "HTTP/1.0 400 Bad request\r\nCache-C"..., 187, MSG_DONTWAIT|MSG_NOSIGNAL) = 187
shutdown(6, 1 /* send */)               = 0

The recv succeeded while epoll_wait() reported an error.

Note: This case is very hard to reproduce and requires that the backend
server is reached via the loopback in order to minimise latency and
reduce the risk of sent data being ACKed.

diff --git a/src/stream_sock.c b/src/stream_sock.c
index afcebb62cfd9b4763a717fd8ecb971f048fdcb9e..fd3748b25edb30f79ed07f7a7b0ff888bdde56fe 100644 (file)
--- a/src/stream_sock.c
+++ b/src/stream_sock.c
@@ -246,8 +246,13 @@ int stream_sock_read(int fd) {
 
        retval = 1;
 
-       /* stop immediately on errors */
-       if (fdtab[fd].state == FD_STERROR || (fdtab[fd].ev & FD_POLL_ERR))
+       /* stop immediately on errors. Note that we DON'T want to stop on
+        * POLL_ERR, as the poller might report a write error while there
+        * are still data available in the recv buffer. This typically
+        * happens when we send too large a request to a backend server
+        * which rejects it before reading it all.
+        */
+       if (fdtab[fd].state == FD_STERROR)
                goto out_error;
 
        /* stop here if we reached the end of data */
@@ -695,7 +700,7 @@ int stream_sock_write(int fd)
 #endif
 
        retval = 1;
-       if (fdtab[fd].state == FD_STERROR || (fdtab[fd].ev & FD_POLL_ERR))
+       if (fdtab[fd].state == FD_STERROR)
                goto out_error;
 
        /* we might have been called just after an asynchronous shutw */