Finally fix #2495

bin/varnishd/cache/cache_hash.c

@@ -63,7 +63,6 @@
 #include "hash/hash_slinger.h"
 
 #include "vsha256.h"
-#include "vtim.h"
 
 struct rush {
 	unsigned		magic;

bin/varnishd/http1/cache_http1_fsm.c

@@ -178,6 +178,51 @@ http1_req_fail(struct req *req, enum sess_close reason)
 		SES_Close(req->sp, reason);
 }
 
+/*----------------------------------------------------------------------
+ */
+
+static int
+http1_req_cleanup(struct sess *sp, struct worker *wrk, struct req *req)
+{
+	AZ(wrk->aws->r);
+	AZ(req->ws->r);
+	Req_Cleanup(sp, wrk, req);
+
+	if (sp->fd >= 0 && req->doclose != SC_NULL)
+		SES_Close(sp, req->doclose);
+
+	if (sp->fd < 0) {
+		wrk->stats->sess_closed++;
+		AZ(req->vcl);
+		Req_Release(req);
+		SES_Delete(sp, SC_NULL, NAN);
+		return (1);
+	}
+
+	return (0);
+}
+
+/*----------------------------------------------------------------------
+ * Clean up a req from waiting list which cannot complete
+ */
+
+static void
+http1_cleanup_waiting(struct worker *wrk, struct req *req,
+    enum sess_close reason)
+{
+	struct sess *sp;
+
+	sp = req->sp;
+	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
+	AN(req->ws->r);
+	WS_Release(req->ws, 0);
+	AN(req->hash_objhead);
+	(void)HSH_DerefObjHead(wrk, &req->hash_objhead);
+	AZ(req->hash_objhead);
+	SES_Close(sp, reason);
+	AN(http1_req_cleanup(sp, wrk, req));
+}
+
 static void v_matchproto_(vtr_reembark_f)
 http1_reembark(struct worker *wrk, struct req *req)
 {
@@ -188,19 +233,15 @@ http1_reembark(struct worker *wrk, struct req *req)
 
 	http1_setstate(sp, H1BUSY);
 
-	if (!SES_Reschedule_Req(req, TASK_QUEUE_REQ))
+	if (!DO_DEBUG(DBG_FAILRESCHED) &&
+	    !SES_Reschedule_Req(req, TASK_QUEUE_REQ))
 		return;
 
 	/* Couldn't schedule, ditch */
 	wrk->stats->busy_wakeup--;
 	wrk->stats->busy_killed++;
-	AN (req->vcl);
-	VCL_Rel(&req->vcl);
-	Req_AcctLogCharge(wrk->stats, req);
-	Req_Release(req);
-	SES_Delete(sp, SC_OVERLOAD, NAN);
-	DSL(DBG_WAITINGLIST, req->vsl->wid, "kill from waiting list");
-	usleep(10000);
+	VSLb(req->vsl, SLT_Error, "Fail to reschedule req from waiting list");
+	http1_cleanup_waiting(wrk, req, SC_OVERLOAD);
 }
 
 static int v_matchproto_(vtr_minimal_response_f)
@@ -324,30 +365,6 @@ http1_dissect(struct worker *wrk, struct req *req)
 	return (0);
 }
 
-/*----------------------------------------------------------------------
- */
-
-static int
-http1_req_cleanup(struct sess *sp, struct worker *wrk, struct req *req)
-{
-	AZ(wrk->aws->r);
-	AZ(req->ws->r);
-	Req_Cleanup(sp, wrk, req);
-
-	if (sp->fd >= 0 && req->doclose != SC_NULL)
-		SES_Close(sp, req->doclose);
-
-	if (sp->fd < 0) {
-		wrk->stats->sess_closed++;
-		AZ(req->vcl);
-		Req_Release(req);
-		SES_Delete(sp, SC_NULL, NAN);
-		return (1);
-	}
-
-	return (0);
-}
-
 /*----------------------------------------------------------------------
  */
 
@@ -477,13 +494,7 @@ HTTP1_Session(struct worker *wrk, struct req *req)
 			 * Check to see if the remote has left.
 			 */
 			if (VTCP_check_hup(sp->fd)) {
-				AN(req->ws->r);
-				WS_Release(req->ws, 0);
-				AN(req->hash_objhead);
-				(void)HSH_DerefObjHead(wrk, &req->hash_objhead);
-				AZ(req->hash_objhead);
-				SES_Close(sp, SC_REM_CLOSE);
-				AN(http1_req_cleanup(sp, wrk, req));
+				http1_cleanup_waiting(wrk, req, SC_REM_CLOSE);
 				return;
 			}
 			http1_setstate(sp, H1PROC);

bin/varnishtest/tests/c00013.vtc

@@ -46,5 +46,64 @@ client c2 {
 
 client c1 -wait
 
+varnish v1 -vsl_catchup
 varnish v1 -expect busy_sleep >= 1
 varnish v1 -expect busy_wakeup >= 1
+varnish v1 -stop
+
+##################################################
+# Now try again where getting a thread fails
+
+barrier b3 cond 2
+barrier b4 cond 2
+
+server s3 {
+	rxreq
+	expect req.url == "/foo"
+	send "HTTP/1.0 200 OK\r\nConnection: close\r\n\r\n"
+	delay .2
+	barrier b3 sync
+	delay .2
+	send "line1\n"
+	delay .2
+	barrier b4 sync
+	send "line2\n"
+} -start
+
+varnish v3 -vcl+backend {
+	sub vcl_backend_fetch {
+		set bereq.backend = s3;
+	}
+	sub vcl_backend_response {
+		set beresp.do_stream = false;
+	}
+} -start
+
+varnish v3 -cliok "param.set debug +failresched"
+
+varnish v3 -cliok "param.set debug +syncvsl"
+
+client c3 -connect ${v3_sock} {
+	txreq -url "/foo" -hdr "client: c3"
+	rxresp
+	expect resp.status == 200
+	expect resp.bodylen == 12
+	expect resp.http.x-varnish == "1001"
+} -start
+
+barrier b3 sync
+
+client c4 -connect ${v3_sock} {
+	txreq -url "/foo" -hdr "client: c4"
+	delay .2
+	barrier b4 sync
+	expect_close
+} -run
+
+client c3 -wait
+
+varnish v1 -vsl_catchup
+varnish v3 -expect busy_sleep >= 1
+varnish v3 -expect busy_wakeup == 0
+varnish v3 -expect busy_killed == 1
+varnish v3 -expect sc_overload == 1

include/tbl/debug_bits.h

@@ -50,6 +50,7 @@ DEBUG_BIT(H2_NOCHECK,		h2_nocheck,	"Disable various H2 checks")
 DEBUG_BIT(VMOD_SO_KEEP,		vmod_so_keep,	"Keep copied VMOD libraries")
 DEBUG_BIT(PROCESSORS,		processors,	"Fetch/Deliver processors")
 DEBUG_BIT(PROTOCOL,		protocol,	"Protocol debugging")
+DEBUG_BIT(FAILRESCHED,		failresched,	"Fail from waiting list")
 #undef DEBUG_BIT
 
 /*lint -restore */