Close race condition on HTTP1 session step counter

When going on a waitinglist, the HTTP1 step counter was set to
S_STP_H1BUSY to allow conditions to be checked on return specific to
having been on a waitinglist. Though the step counter was set after
entering the waitinglist, and if the session was rescheduled
immediately on another thread a race opened which would mess up the
state handling.

Fix this by elliminating the S_STP_H1BUSY step, and having condition
checks on req->hash_objhead in the S_STP_H1PROC state to handle the
waitinglist return specific checks.

The panic output has been changed to include the req->hash_objhead
pointer if set. This exposes the waitinglist condition in the panic
output which would otherwise be hidden by the removal of the
S_STP_H1BUSY step.

Fixes: #2117

bin/varnishd/cache/cache_panic.c

@@ -345,6 +345,8 @@ pan_req(struct vsb *vsb, const struct req *req)
 		VSB_printf(vsb, "step = %s,\n", stp);
 	else
 		VSB_printf(vsb, "step = 0x%x,\n", req->req_step);
+	if (req->hash_objhead)
+		VSB_printf(vsb, "hash_objhead = %p\n", req->hash_objhead);
 
 	VSB_printf(vsb, "req_body = %s,\n",
 	    reqbody_status_2str(req->req_body_status));

bin/varnishd/http1/cache_http1_fsm.c

@@ -247,27 +247,22 @@ HTTP1_Session(struct worker *wrk, struct req *req)
 			req->req_step = R_STP_RECV;
 			sp->sess_step = S_STP_H1PROC;
 			break;
-		case S_STP_H1BUSY:
-			/*
-			 * Return from waitinglist.
-			 * Check to see if the remote has left.
-			 */
-			if (VTCP_check_hup(sp->fd)) {
-				AN(req->hash_objhead);
+		case S_STP_H1PROC:
+			req->transport = &http1_transport;
+			req->task.func = SES_Proto_Req;
+			req->task.priv = req;
+			if (req->hash_objhead && VTCP_check_hup(sp->fd)) {
+				/* Return from waitinglist and the remote
+				   has left. */
 				(void)HSH_DerefObjHead(wrk, &req->hash_objhead);
 				AZ(req->hash_objhead);
 				SES_Close(sp, SC_REM_CLOSE);
 				AN(Req_Cleanup(sp, wrk, req));
 				return;
 			}
-			sp->sess_step = S_STP_H1PROC;
-			break;
-		case S_STP_H1PROC:
-			req->transport = &http1_transport;
-			req->task.func = SES_Proto_Req;
-			req->task.priv = req;
 			if (CNT_Request(wrk, req) == REQ_FSM_DISEMBARK) {
-				sp->sess_step = S_STP_H1BUSY;
+				/* Have been placed on waitinglist. Any
+				   changes to req and sp are unsafe. */
 				return;
 			}
 			req->transport = NULL;

include/tbl/steps.h

@@ -34,7 +34,6 @@
 SESS_STEP(h1newsess,	H1NEWSESS)
 SESS_STEP(h1newreq,	H1NEWREQ)
 SESS_STEP(h1proc,	H1PROC)
-SESS_STEP(h1busy,	H1BUSY)
 SESS_STEP(h1cleanup,	H1CLEANUP)
 SESS_STEP(h1_last,	H1_LAST)