Revert "MEDIUM: queue: move the queue lock manipulation to pendconn_process_next_strm()"

This reverts commit 9a6d0ddbd6788a05c6730bf0e9e8550d7c5b11aa.

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 8db2da2c04568cba8bc4bbc079692c67528e3a7d..5c9db172544d8e328bbe71731787c8ada8c55685 100644 (file)
--- a/src/queue.c
+++ b/src/queue.c
@@ -255,8 +255,8 @@ static struct pendconn *pendconn_first(struct eb_root *pendconns)
  *
  * The proxy's queue will be consulted only if px_ok is non-zero.
  *
- * This function uses both the proxy and the server queues' lock. Today it is
- * only called by process_srv_queue.
+ * 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.
  *
  * 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
@@ -270,20 +270,15 @@ static struct pendconn *pendconn_process_next_strm(struct server *srv, struct pr
        u32 pkey, ppkey;
 
        p = NULL;
-       HA_SPIN_LOCK(QUEUE_LOCK, &srv->queue.lock);
        if (srv->queue.length)
                p = pendconn_first(&srv->queue.head);
 
        pp = NULL;
-       HA_SPIN_LOCK(QUEUE_LOCK, &px->queue.lock);
        if (px_ok && px->queue.length)
                pp = pendconn_first(&px->queue.head);
 
-       if (!p && !pp) {
-               HA_SPIN_UNLOCK(QUEUE_LOCK, &px->queue.lock);
-               HA_SPIN_UNLOCK(QUEUE_LOCK, &srv->queue.lock);
+       if (!p && !pp)
                return NULL;
-       }
        else if (!pp)
                goto use_p; /*  p != NULL */
        else if (!p)
@@ -312,17 +307,13 @@ 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 */
        __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:
-       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);
        _HA_ATOMIC_DEC(&srv->queue.length);
        _HA_ATOMIC_DEC(&px->totpend);
@@ -356,8 +347,14 @@ void process_srv_queue(struct server *s)
        while (s->served < maxconn) {
                struct pendconn *pc;
 
+               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);
 
+               HA_SPIN_UNLOCK(QUEUE_LOCK, &p->queue.lock);
+               HA_SPIN_UNLOCK(QUEUE_LOCK, &s->queue.lock);
+
                if (!pc)
                        break;