r2401 - trunk/varnish-cache/bin/varnishd

[phk at projects.linpro.no]

Author: phk
Date: 2008-01-29 17:05:54 +0100 (Tue, 29 Jan 2008)
New Revision: 2401

Modified:
   trunk/varnish-cache/bin/varnishd/cache_acceptor_kqueue.c
Log:
I am not sure if this is a/the race some users are seeing, or if it
even can have any effect, but this will close it at a cost of one
extra kevent(2) every 100ms timer tick.

The (perceived) problem is that we have pending kqueue changes we
have not yet told the kernel about, then close a number of expired
FD's which might be instantly be recycled by the accept(2) over in
the other thread before we tell the kernel about the pending changes.

In that case, the kernel has no way of knowing that our changes
referred to the previous instance of the fd and not the new one.

The solution is to push the changes to the kernel before servicing
the timer.



Modified: trunk/varnish-cache/bin/varnishd/cache_acceptor_kqueue.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/cache_acceptor_kqueue.c	2008-01-29 11:53:41 UTC (rev 2400)
+++ trunk/varnish-cache/bin/varnishd/cache_acceptor_kqueue.c	2008-01-29 16:05:54 UTC (rev 2401)
@@ -140,33 +140,41 @@
 static unsigned nki;
 
 static void
-vca_kq_sess(struct sess *sp, short arm)
+vca_kq_flush(void)
 {
 	int i;
 
+	if (nki == 0)
+		return;
+	i = kevent(kq, ki, nki, NULL, 0, NULL);
+	assert(i <= 0);
+	if (i < 0) {
+		/* 
+		 * We do not push kevents into the kernel before 
+		 * passing the session off to a worker thread, so
+		 * by the time we get around to delete the event
+		 * the fd may be closed and we get an ENOENT back
+		 * once we do flush.
+		 * We can get EBADF the same way if the client closes
+		 * on us.  In that case, we get no kevent on that
+		 * socket, but the TAILQ still has it, and it will
+		 * be GC'ed there after the timeout.
+		 */
+		assert(errno == ENOENT || errno == EBADF);
+	}
+	nki = 0;
+}
+
+static void
+vca_kq_sess(struct sess *sp, short arm)
+{
+
 	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
 	if (sp->fd < 0)
 		return;
 	EV_SET(&ki[nki], sp->fd, EVFILT_READ, arm, 0, 0, sp);
-	if (++nki == NKEV) {
-		i = kevent(kq, ki, nki, NULL, 0, NULL);
-		assert(i <= 0);
-		if (i < 0) {
-			/* 
-			 * We do not push kevents into the kernel before 
-			 * passing the session off to a worker thread, so
-			 * by the time we get around to delete the event
-			 * the fd may be closed and we get an ENOENT back
-			 * once we do flush.
-			 * We can get EBADF the same way if the client closes
-			 * on us.  In that case, we get no kevent on that
-			 * socket, but the TAILQ still has it, and it will
-			 * be GC'ed there after the timeout.
-			 */
-			assert(errno == ENOENT || errno == EBADF);
-		}
-		nki = 0;
-	}
+	if (++nki == NKEV) 
+		vca_kq_flush();
 }
 
 static void
@@ -269,6 +277,14 @@
 		}
 		if (!dotimer)
 			continue;
+		/*
+		 * Make sure we have no pending changes for the fd's
+		 * we are about to close, in case the accept(2) in the
+		 * other thread creates new fd's betwen our close and
+		 * the kevent(2) at the top of this loop, the kernel
+		 * would not know we meant "the old fd of this number".
+		 */
+		vca_kq_flush();
 		deadline = TIM_real() - params->sess_timeout;
 		for (;;) {
 			sp = VTAILQ_FIRST(&sesshead);