Be more consistent about sockets and blocking/non-blocking mode:

Accept sockets are non-blocking, to avoid races where the client closes
before we get to accept it.  (Spotted by: "chen xiaoyong")

Unfortunately, that means that session sockets inherit non-blocking mode,
which is the opposite of what we want in the worker thread but correct
for the acceptor thread.

We prefer to have the extra syscalls in the worker thread, that complicates
things a little bit.

Use ioctl(FIONBIO) instead of fcntl(2) which is surprisingly expensive.


git-svn-id: http://www.varnish-cache.org/svn/trunk/varnish-cache@2639 d4fa192b-c00b-0410-8231-f00ffab90ce4

bin/varnishd/cache_acceptor.c

@@ -270,6 +270,11 @@ vca_return_session(struct sess *sp)
 	AZ(sp->obj);
 	AZ(sp->vcl);
 	assert(sp->fd >= 0);
+	/*
+	 * Set nonblocking in the worker-thread, before passing to the
+	 * acceptor thread, to reduce syscall density of the latter.
+	 */
+	TCP_nonblocking(sp->fd);
 	if (vca_act->pass == NULL)
 		assert(sizeof sp == write(vca_pipes[1], &sp, sizeof sp));
 	else

bin/varnishd/cache_center.c

@@ -906,6 +906,16 @@ CNT_Session(struct sess *sp)
 	w = sp->wrk;
 	CHECK_OBJ_NOTNULL(w, WORKER_MAGIC);
 
+	/*
+	 * Whenever we come in from the acceptor we need to set blocking
+	 * mode, but there is no point in setting it when we come from
+	 * ESI or when a parked sessions returns.
+	 * It would be simpler to do this in the acceptor, but we'd rather
+	 * do the syscall in the worker thread.
+ 	 */
+	if (sp->step == STP_FIRST || sp->step == STP_START)
+		TCP_blocking(sp->fd);
+
 	for (done = 0; !done; ) {
 		assert(sp->wrk == w);
 		/*

bin/varnishd/mgt_child.c

@@ -150,6 +150,12 @@ open_sockets(void)
 			free(ls);
 			continue;
 		}
+		/*
+		 * Set nonblocking mode to avoid a race where a client
+		 * closes before we call accept(2) and nobody else are in
+		 * the listen queue to release us.
+		 */
+		TCP_nonblocking(ls->sock);
 		TCP_filter_http(ls->sock);
 		good++;
 	}

bin/varnishd/tcp.c

@@ -37,7 +37,7 @@
 #include <netinet/in.h>
 
 #include <errno.h>
-#include <fcntl.h>
+#include <sys/ioctl.h>
 #include <netdb.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -112,18 +112,22 @@ TCP_filter_http(int sock)
 #endif
 }
 
-/*--------------------------------------------------------------------*/
+/*--------------------------------------------------------------------
+ * Functions for controlling NONBLOCK mode.
+ * 
+ * We use FIONBIO because it is cheaper than fcntl(2), which requires
+ * us to do two syscalls, one to get and one to set, the latter of
+ * which mucks about a bit before it ends up calling ioctl(FIONBIO),
+ * at least on FreeBSD.
+ */
 
 void
 TCP_blocking(int sock)
 {
 	int i;
 
-	i = fcntl(sock, F_GETFL);
-	assert(i != -1);
-	i &= ~O_NONBLOCK;
-	i = fcntl(sock, F_SETFL, i);
-	assert(i != -1);
+	i = 0;
+	AZ(ioctl(sock, FIONBIO, &i));
 }
 
 void
@@ -131,9 +135,6 @@ TCP_nonblocking(int sock)
 {
 	int i;
 
-	i = fcntl(sock, F_GETFL);
-	assert(i != -1);
-	i |= O_NONBLOCK;
-	i = fcntl(sock, F_SETFL, i);
-	assert(i != -1);
+	i = 1;
+	AZ(ioctl(sock, FIONBIO, &i));
 }