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: svn+ssh://projects.linpro.no/svn/varnish/trunk@2639 d4fa192b-c00b-0410-8231-f00ffab90ce4

diff --git a/varnish-cache/bin/varnishd/cache_acceptor.c b/varnish-cache/bin/varnishd/cache_acceptor.c
index 6394cee03b7c0d00bbb14ed3f613f74949f3d4db..28e18bcda31cfd54467eb65527c7905fbebce6a5 100644 (file)
--- a/varnish-cache/bin/varnishd/cache_acceptor.c
+++ b/varnish-cache/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

diff --git a/varnish-cache/bin/varnishd/cache_center.c b/varnish-cache/bin/varnishd/cache_center.c
index 2ae2dcbc1cccdabb4b3fae7ee8b46c50727120e0..1f859c214a75a39cd429a5839e36f6c9fdc86622 100644 (file)
--- a/varnish-cache/bin/varnishd/cache_center.c
+++ b/varnish-cache/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);
                /*

diff --git a/varnish-cache/bin/varnishd/mgt_child.c b/varnish-cache/bin/varnishd/mgt_child.c
index 717833b700018c60e3229584944f3f89d4770462..7b88d3048d5260be7e58c58f33fc09a0b6d9da6a 100644 (file)
--- a/varnish-cache/bin/varnishd/mgt_child.c
+++ b/varnish-cache/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++;
        }

diff --git a/varnish-cache/bin/varnishd/tcp.c b/varnish-cache/bin/varnishd/tcp.c
index 0d1d7d628b1b07a2d87f405da8dbd552e5599470..f0c9a9660793824b901f4b6d2d0dd479429f7767 100644 (file)
--- a/varnish-cache/bin/varnishd/tcp.c
+++ b/varnish-cache/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));
 }