BUG/MAJOR: queue: lock around the call to pendconn_process_next_strm()

The extra call to pendconn_process_next_strm() made in commit cda7275ef5
("MEDIUM: queue: Handle the race condition between queue and dequeue
differently") was performed after releasing the server queue's lock,
which is incompatible with the calling convention for this function.
The result is random corruption of the server's streams list likely
due to picking old or incorrect pendconns from the queue, and in the
end infinitely looping on apparently already locked mt_list objects.
Just adding the lock fixes the problem.

It's very difficult to reproduce, it requires low maxconn values on
servers, stickiness on the servers (cookie), a long enough slowstart
(e.g. 10s), and regularly flipping servers up/down to re-trigger the
slowstart.

No backport is needed as this was only in 3.2.

diff --git a/src/queue.c b/src/queue.c
index 62832f07ae692c006a4b048555c591e7adbca244..d5f737bf5cb2f117db0ae724e8eb10de51284ec7 100644 (file)
--- a/src/queue.c
+++ b/src/queue.c
@@ -513,12 +513,15 @@ int process_srv_queue(struct server *s)
                 * just in case try to run one more stream.
                 */
                for (i = 0; i < global.nbtgroups; i++) {
+                       HA_SPIN_LOCK(QUEUE_LOCK, &s->per_tgrp[i].queue.lock);
                        if (pendconn_process_next_strm(s, p, px_ok, i + 1)) {
+                               HA_SPIN_UNLOCK(QUEUE_LOCK, &s->per_tgrp[i].queue.lock);
                                _HA_ATOMIC_SUB(&p->totpend, 1);
                                _HA_ATOMIC_ADD(&p->served, 1);
                                done++;
                                break;
                        }
+                       HA_SPIN_UNLOCK(QUEUE_LOCK, &s->per_tgrp[i].queue.lock);
                }
        }
        return done;