From: Willy Tarreau <w@1wt.eu>
Date: Thu, 24 Jun 2021 05:21:59 +0000 (+0200)
Subject: Revert "MEDIUM: queue: unlock as soon as possible"
X-Git-Tag: v2.5-dev1~41
X-Git-Url: http://git.haproxy.org/?a=commitdiff_plain;h=90a160a465a3e049a5bd36fe1667af77b2c2ae25;p=haproxy-2.5.git

Revert "MEDIUM: queue: unlock as soon as possible"

This reverts commit 5b3927531145384bad8d2b46ca21f017afde81c7.

The recent changes since 5304669e1 MEDIUM: queue: make
pendconn_process_next_strm() only return the pendconn opened a tiny race
condition between stream_free() and process_srv_queue(), as the pendconn
is accessed outside of the lock, possibly while it's being freed. A
different approach is required.
---

diff --git a/src/queue.c b/src/queue.c
index e16331f..8db2da2 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -270,23 +270,20 @@ static struct pendconn *pendconn_process_next_strm(struct server *srv, struct pr
 	u32 pkey, ppkey;
 
 	p = NULL;
-	if (srv->queue.length) {
-		HA_SPIN_LOCK(QUEUE_LOCK, &srv->queue.lock);
+	HA_SPIN_LOCK(QUEUE_LOCK, &srv->queue.lock);
+	if (srv->queue.length)
 		p = pendconn_first(&srv->queue.head);
-		if (!p)
-			HA_SPIN_UNLOCK(QUEUE_LOCK, &srv->queue.lock);
-	}
 
 	pp = NULL;
-	if (px_ok && px->queue.length) {
-		HA_SPIN_LOCK(QUEUE_LOCK, &px->queue.lock);
+	HA_SPIN_LOCK(QUEUE_LOCK, &px->queue.lock);
+	if (px_ok && px->queue.length)
 		pp = pendconn_first(&px->queue.head);
-		if (!pp)
-			HA_SPIN_UNLOCK(QUEUE_LOCK, &px->queue.lock);
-	}
 
-	if (!p && !pp)
+	if (!p && !pp) {
+		HA_SPIN_UNLOCK(QUEUE_LOCK, &px->queue.lock);
+		HA_SPIN_UNLOCK(QUEUE_LOCK, &srv->queue.lock);
 		return NULL;
+	}
 	else if (!pp)
 		goto use_p; /*  p != NULL */
 	else if (!p)
@@ -314,18 +311,16 @@ static struct pendconn *pendconn_process_next_strm(struct server *srv, struct pr
 
  use_pp:
 	/* Let's switch from the server pendconn to the proxy pendconn */
-	if (p)
-		HA_SPIN_UNLOCK(QUEUE_LOCK, &srv->queue.lock);
 	__pendconn_unlink_prx(pp);
 	HA_SPIN_UNLOCK(QUEUE_LOCK, &px->queue.lock);
+	HA_SPIN_UNLOCK(QUEUE_LOCK, &srv->queue.lock);
 	_HA_ATOMIC_INC(&px->queue.idx);
 	_HA_ATOMIC_DEC(&px->queue.length);
 	_HA_ATOMIC_DEC(&px->totpend);
 	p = pp;
 	goto unlinked;
  use_p:
-	if (pp)
-		HA_SPIN_UNLOCK(QUEUE_LOCK, &px->queue.lock);
+	HA_SPIN_UNLOCK(QUEUE_LOCK, &px->queue.lock);
 	__pendconn_unlink_srv(p);
 	HA_SPIN_UNLOCK(QUEUE_LOCK, &srv->queue.lock);
 	_HA_ATOMIC_INC(&srv->queue.idx);