[pkg-lighttpd] Bug#541428: lighttpd doesn't do linger-on-close correctly, messing up pipelining clients

Avery Pennarun apenwarr at gmail.com
Fri Aug 14 05:30:35 UTC 2009


Package: lighttpd
Version: 1.4.19-5
Severity: normal
Tags: patch

Hi,

I found a bug in lighttpd tonight relating to pipelining requests.  If you
make more requests on a single connection than the max-keep-alive-requests
(default 16), and your client is pipelining, *and* you're unlucky, it'll
sometimes RST your connection and give you bad data.

This is probably related to (and may be the cause of) the lighttpd bug
reported here: http://redmine.lighttpd.net/issues/657

Attached below is a patch which fixes the problem, as well as giving a more
thorough explanation of what's going on.

Have fun,

Avery


-- System Information:
Debian Release: 4.0
  APT prefers stable
  APT policy: (500, 'stable')
Architecture: i386 (i686)

Kernel: Linux 2.6.18-6-686 (SMP w/2 CPU cores)
Locale: LANG=en_CA.UTF-8, LC_CTYPE=en_CA.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash

Versions of packages lighttpd depends on:
ii  libattr1               1:2.4.43-2        Extended attribute shared library
ii  libbz2-1.0             1.0.5-1           high-quality block-sorting file co
ii  libc6                  2.7-18            GNU C Library: Shared libraries
ii  libfam0                2.7.0-13.3        Client library to control the FAM 
ii  libldap-2.4-2          2.4.11-1          OpenLDAP libraries
ii  libpcre3               7.6-2.1           Perl 5 Compatible Regular Expressi
ii  libssl0.9.8            0.9.8g-15+lenny1  SSL shared libraries
ii  libterm-readline-perl- 1.0302-1          Perl implementation of Readline li
ii  lsb-base               3.2-20            Linux Standard Base 3.2 init scrip
ii  mime-support           3.39-1            MIME files 'mime.types' & 'mailcap
ii  zlib1g                 1:1.2.3.3.dfsg-12 compression library - runtime

lighttpd recommends no packages.

Versions of packages lighttpd suggests:
ii  apache2-utils              2.2.3-4+etch6 utility programs for webservers
ii  openssl                    0.9.8c-4etch3 Secure Socket Layer (SSL) binary a
pn  rrdtool                    <none>        (no description available)

-- no debconf information


commit 9ecb4c72816acd71f810c8d6db88cf829027fc10
Author: Avery Pennarun <apenwarr at gmail.com>
Date:   Thu Aug 13 22:06:31 2009 -0400

    Fix linger-on-close behaviour to avoid rare failure conditions.
    
    - Don't assume that when FIONREAD returns 0, that it's safe to close the
      socket.  There may still be data that's about to arrive, and we'll still
      send an RST if the socket is confused, potentially confusing the client.
    
    - Don't close the connection immediately after sending a successful
      response; linger-on-close was only happening in the case of errors, but it
      has to happen in case of success too, because the client doesn't
      necessarily know we're about to close after this request, and may have
      sent additional ones. (eg. if server.max-keep-alive-requests is small.)
    
    - Don't close the connection immediately even if keep_alive is 0; there are
      several reasons keep_alive can be 0.  If the client requested Connection:
      close, then it would be okay to close right away, since we can assume he
      didn't send anything else.  But it's harmless (and more resilient) to do
      the lingering regardless.
    
    - Increase the lingering timeout from 1s to 30s.  In the vast majority of
      cases, the timeout never kicks in anyway.  The only times when it might
      be needed are a) in race conditions, in which case timing out too early
      defeats the purpose of lingering at all; b) if there's a lot of data,
      which is basically the same as (a); or c) if the remote end disappears,
      in which case we now suffer through a longer timeout... but we would
      anyway, if we were waiting for them to receive our transmission.

diff --git a/src/connections.c b/src/connections.c
index d547b65..fbbdf40 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -1255,9 +1255,11 @@ handler_t connection_handle_fdevent(void *s, void *context, int revents) {
 			/* */
 			read(con->fd, buf, sizeof(buf));
 		} else {
-			/* nothing to read */
-
-			con->close_timeout_ts = 0;
+			/* nothing to read - yet.  But that doesn't
+			 * mean something won't show up in our buffers
+			 * sometime soon, so we can't quite until
+			 * poll() gives us the HUP notification.
+			 */
 		}
 	}
 
@@ -1569,7 +1571,12 @@ int connection_state_machine(server *srv, connection *con) {
 					}
 				}
 #endif
-				connection_close(srv, con);
+				if ((0 == shutdown(con->fd, SHUT_WR))) {
+					con->close_timeout_ts = srv->cur_ts;
+					connection_set_state(srv, con, CON_STATE_CLOSE);
+				} else {
+					connection_close(srv, con);
+				}
 
 				srv->con_closed++;
 			}
@@ -1594,28 +1601,31 @@ int connection_state_machine(server *srv, connection *con) {
 						"state for fd", con->fd, connection_get_state(con->state));
 			}
 
-			if (con->keep_alive) {
-				if (ioctl(con->fd, FIONREAD, &b)) {
-					log_error_write(srv, __FILE__, __LINE__, "ss",
-							"ioctl() failed", strerror(errno));
-				}
-				if (b > 0) {
-					char buf[1024];
-					log_error_write(srv, __FILE__, __LINE__, "sdd",
-							"CLOSE-read()", con->fd, b);
-
-					/* */
-					read(con->fd, buf, sizeof(buf));
-				} else {
-					/* nothing to read */
+			/* we have to do the linger_on_close stuff regardless
+			 * of con->keep_alive; even non-keepalive sockets may
+			 * still have unread data, and closing before reading
+			 * it will make the client not see all our output.
+			 */
+			if (ioctl(con->fd, FIONREAD, &b)) {
+				log_error_write(srv, __FILE__, __LINE__, "ss",
+					"ioctl() failed", strerror(errno));
+			}
+			if (b > 0) {
+				char buf[1024];
+				log_error_write(srv, __FILE__, __LINE__, "sdd",
+						"CLOSE-read()", con->fd, b);
 
-					con->close_timeout_ts = 0;
-				}
+				/* */
+				read(con->fd, buf, sizeof(buf));
 			} else {
-				con->close_timeout_ts = 0;
+				/* nothing to read - yet.  But that doesn't
+				 * mean something won't show up in our buffers
+				 * sometime soon, so we can't quite until
+				 * poll() gives us the HUP notification.
+				 */
 			}
 
-			if (srv->cur_ts - con->close_timeout_ts > 1) {
+			if (srv->cur_ts - con->close_timeout_ts > 30) {
 				connection_close(srv, con);
 
 				if (srv->srvconf.log_state_handling) {
@@ -1739,8 +1749,7 @@ int connection_state_machine(server *srv, connection *con) {
 			connection_reset(srv, con);
 
 			/* close the connection */
-			if ((con->keep_alive == 1) &&
-			    (0 == shutdown(con->fd, SHUT_WR))) {
+			if ((0 == shutdown(con->fd, SHUT_WR))) {
 				con->close_timeout_ts = srv->cur_ts;
 				connection_set_state(srv, con, CON_STATE_CLOSE);
 





More information about the pkg-lighttpd-maintainers mailing list