From: Willy Tarreau <w@1wt.eu>
Date: Thu, 24 Jun 2021 05:22:08 +0000 (+0200)
Subject: Revert "MEDIUM: queue: determine in process_srv_queue() if the proxy is usable"
X-Git-Tag: v2.5-dev1~39
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ddac4a1f358e2620e4c8c4b2845a8a83eaf47d71;p=thirdparty%2Fhaproxy.git

Revert "MEDIUM: queue: determine in process_srv_queue() if the proxy is usable"

This reverts commit de814dd4228fa5e528d9ac1f0e1c4c7325ee52d3.

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 5c9db17254..20880a2390 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -253,28 +253,34 @@ static struct pendconn *pendconn_first(struct eb_root *pendconns)
  * immediately marked as "assigned", and both its <srv> and <srv_conn> are set
  * to <srv>.
  *
- * The proxy's queue will be consulted only if px_ok is non-zero.
- *
  * This function must only be called if the server queue _AND_ the proxy queue
- * are locked (if pk_ok is set). Today it is only called by process_srv_queue.
+ * are locked. Today it is only called by process_srv_queue.
  *
  * The function returns the dequeued pendconn on success or NULL if none is
  * available. It's up to the caller to add the corresponding stream to the
  * server's list, to update the LB algo, update ->served, and to wake up the
  * stream's task.
  */
-static struct pendconn *pendconn_process_next_strm(struct server *srv, struct proxy *px, int px_ok)
+static struct pendconn *pendconn_process_next_strm(struct server *srv, struct proxy *px)
 {
 	struct pendconn *p = NULL;
 	struct pendconn *pp = NULL;
+	struct server   *rsrv;
 	u32 pkey, ppkey;
 
+	rsrv = srv->track;
+	if (!rsrv)
+		rsrv = srv;
+
 	p = NULL;
 	if (srv->queue.length)
 		p = pendconn_first(&srv->queue.head);
 
 	pp = NULL;
-	if (px_ok && px->queue.length)
+	if (srv_currently_usable(rsrv) && px->queue.length &&
+	    (!(srv->flags & SRV_F_BACKUP) ||
+	     (!px->srv_act &&
+	      (srv == px->lbprm.fbck || (px->options & PR_O_USE_ALL_BK)))))
 		pp = pendconn_first(&px->queue.head);
 
 	if (!p && !pp)
@@ -329,19 +335,9 @@ static struct pendconn *pendconn_process_next_strm(struct server *srv, struct pr
  */
 void process_srv_queue(struct server *s)
 {
-	struct server *ref = s->track ? s->track : s;
 	struct proxy  *p = s->proxy;
 	int done = 0;
 	int maxconn;
-	int px_ok;
-
-	/* if a server is not usable or backup and must not be used
-	 * to dequeue backend requests.
-	 */
-	px_ok = srv_currently_usable(ref) &&
-	        (!(s->flags & SRV_F_BACKUP) ||
-	         (!p->srv_act &&
-	          (s == p->lbprm.fbck || (p->options & PR_O_USE_ALL_BK))));
 
 	maxconn = srv_dynamic_maxconn(s);
 	while (s->served < maxconn) {
@@ -350,7 +346,7 @@ void process_srv_queue(struct server *s)
 		HA_SPIN_LOCK(QUEUE_LOCK, &s->queue.lock);
 		HA_SPIN_LOCK(QUEUE_LOCK, &p->queue.lock);
 
-		pc = pendconn_process_next_strm(s, p, px_ok);
+		pc = pendconn_process_next_strm(s, p);
 
 		HA_SPIN_UNLOCK(QUEUE_LOCK, &p->queue.lock);
 		HA_SPIN_UNLOCK(QUEUE_LOCK, &s->queue.lock);