[PATCH 2/2] On return from waitinglist, do a poll() on the socket to see if the client has closed the connection and gone away. If so, release the session early.

[Martin Blix Grydeland]

Fixes: #1252
---
 bin/varnishd/cache/cache_hash.c           |   25 +++++++++++++---
 bin/varnishd/cache/cache_http1_fsm.c      |   14 +++++++++
 bin/varnishd/hash/hash_slinger.h          |    1 +
 bin/varnishtest/tests.disabled/r01252.vtc |   44 +++++++++++++++++++++++++++++
 include/vtcp.h                            |    1 +
 lib/libvarnish/vtcp.c                     |   19 +++++++++++++
 6 files changed, 100 insertions(+), 4 deletions(-)
 create mode 100644 bin/varnishtest/tests.disabled/r01252.vtc

diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index d0d987e..db906ec 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -399,7 +399,7 @@ HSH_Lookup(struct req *req)
 		assert(oc->objhead == oh);
 		oc->refcnt++;
 		Lck_Unlock(&oh->mtx);
-		assert(hash->deref(oh));
+		assert(HSH_DerefObjHead(&wrk->stats, &oh));
 		o = oc_getobj(&wrk->stats, oc);
 		CHECK_OBJ_NOTNULL(o, OBJECT_MAGIC);
 		if (!cache_param->obj_readonly && o->hits < INT_MAX)
@@ -694,13 +694,30 @@ HSH_Deref(struct dstat *ds, struct objcore *oc, struct object **oo)
 	if (oh != NULL) {
 		/* Drop our ref on the objhead */
 		assert(oh->refcnt > 0);
-		if (hash->deref(oh))
-			return (0);
-		HSH_DeleteObjHead(ds, oh);
+		(void)HSH_DerefObjHead(ds, &oh);
 	}
 	return (0);
 }
 
+int
+HSH_DerefObjHead(struct dstat *ds, struct objhead **poh)
+{
+	struct objhead *oh;
+	int r;
+
+	AN(ds);
+	AN(poh);
+	oh = *poh;
+	*poh = NULL;
+	CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
+
+	assert(oh->refcnt > 0);
+	r = hash->deref(oh);
+	if (!r)
+		HSH_DeleteObjHead(ds, oh);
+	return (r);
+}
+
 void
 HSH_Init(const struct hash_slinger *slinger)
 {
diff --git a/bin/varnishd/cache/cache_http1_fsm.c b/bin/varnishd/cache/cache_http1_fsm.c
index 2f99c6e..ecc3583 100644
--- a/bin/varnishd/cache/cache_http1_fsm.c
+++ b/bin/varnishd/cache/cache_http1_fsm.c
@@ -74,6 +74,7 @@
 #include <stdlib.h>
 
 #include "cache.h"
+#include "hash/hash_slinger.h"
 
 #include "vcl.h"
 #include "vct.h"
@@ -329,6 +330,19 @@ HTTP1_Session(struct worker *wrk, struct req *req)
 		return;
 	}
 
+	/*
+	 * Return from waitinglist. Check to see if the remote has left.
+	 */
+	if (req->req_step == R_STP_LOOKUP && VTCP_check_hup(sp->fd)) {
+		AN(req->hash_objhead);
+		(void)HSH_DerefObjHead(&wrk->stats, &req->hash_objhead);
+		AZ(req->hash_objhead);
+		SES_Close(sp, SC_REM_CLOSE);
+		sdr = http1_cleanup(sp, wrk, req);
+		assert(sdr == SESS_DONE_RET_GONE);
+		return;
+	}
+
 	if (sp->sess_step == S_STP_NEWREQ) {
 		HTTP1_Init(req->htc, req->ws, sp->fd, req->vsl,
 		    cache_param->http_req_size,
diff --git a/bin/varnishd/hash/hash_slinger.h b/bin/varnishd/hash/hash_slinger.h
index c385ea6..f90f592 100644
--- a/bin/varnishd/hash/hash_slinger.h
+++ b/bin/varnishd/hash/hash_slinger.h
@@ -98,6 +98,7 @@ struct objhead {
 void HSH_Unbusy(struct dstat *, struct objcore *);
 void HSH_Complete(struct objcore *oc);
 void HSH_DeleteObjHead(struct dstat *, struct objhead *oh);
+int HSH_DerefObjHead(struct dstat *, struct objhead **poh);
 int HSH_Deref(struct dstat *, struct objcore *oc, struct object **o);
 #endif /* VARNISH_CACHE_CHILD */
 
diff --git a/bin/varnishtest/tests.disabled/r01252.vtc b/bin/varnishtest/tests.disabled/r01252.vtc
new file mode 100644
index 0000000..c988da6
--- /dev/null
+++ b/bin/varnishtest/tests.disabled/r01252.vtc
@@ -0,0 +1,44 @@
+varnishtest "#1252 - Drop remote closed connections returning from waitinglists"
+
+# This test case is disabled because it will only pass on platforms
+# where the tcp_keepalive_* runtime arguments are available, and also
+# because it requires "-t 75" argument to varnishtest (remote closed
+# state will only be detected after FIN timeout has passed (60s))
+
+server s1 {
+	rxreq
+	expect req.http.X-Client == "1"
+	sema r1 sync 2
+	delay 75
+	close
+} -start
+
+server s2 {
+	rxreq
+	expect req.url == "/should/not/happen"
+	txresp
+} -start
+
+varnish v1 -arg "-p debug=+waitinglist -p tcp_keepalive_time=1s -p tcp_keepalive_probes=1 -p tcp_keepalive_intvl=1s -p first_byte_timeout=70" -vcl+backend {
+	sub vcl_recv {
+		if (req.http.x-client == "2") {
+			set req.backend = s2;
+		}
+	}
+} -start
+
+client c1 {
+	timeout 70
+	txreq -hdr "X-Client: 1"
+	rxresp
+	expect resp.status == 503
+} -start
+
+client c2 {
+	sema r1 sync 2
+	txreq -hdr "X-Client: 2"
+	delay 1
+} -start
+
+client c1 -wait
+client c2 -wait
diff --git a/include/vtcp.h b/include/vtcp.h
index 77f86ed..1594a4d 100644
--- a/include/vtcp.h
+++ b/include/vtcp.h
@@ -62,6 +62,7 @@ int VTCP_filter_http(int sock);
 int VTCP_blocking(int sock);
 int VTCP_nonblocking(int sock);
 int VTCP_linger(int sock, int linger);
+int VTCP_check_hup(int sock);
 
 #ifdef SOL_SOCKET
 int VTCP_port(const struct sockaddr_storage *addr);
diff --git a/lib/libvarnish/vtcp.c b/lib/libvarnish/vtcp.c
index 2c6dd3f..7227fb3 100644
--- a/lib/libvarnish/vtcp.c
+++ b/lib/libvarnish/vtcp.c
@@ -308,3 +308,22 @@ VTCP_linger(int sock, int linger)
 	VTCP_Assert(i);
 	return (i);
 }
+
+/*--------------------------------------------------------------------
+ * Do a poll to check for remote HUP
+ */
+
+int
+VTCP_check_hup(int sock)
+{
+	struct pollfd pfd;
+
+	assert(sock > 0);
+	pfd.fd = sock;
+	pfd.events = POLLOUT;
+	pfd.revents = 0;
+
+	if (poll(&pfd, 1, 0) == 1 && pfd.revents & POLLHUP)
+		return (1);
+        return (0);
+}
-- 
1.7.10.4