MINOR: queue: add a function to check for TOCTOU after queueing
There's a rare TOCTOU case that happens from time to time with maxconn 1
and multiple threads. Between the moment we see the queue full and the
moment we queue a request, it's possible that the last request on the
server or proxy ended and that no other one is left to offer it its place.
Given that all this code path is performance-critical and we cannot afford
to increase the lock duration, better recheck for the condition after
queueing. For this we need to be able to check for the condition and
cleanly dequeue a request. That's what this patch provides via the new
function pendconn_must_try_again(). It will catch more requests than
absolutely needed though it will catch them all. It may find that around
1/1000 of requests are at risk, though testing shows that in practice,
it's around 1 per million that really gets stuck (other ones benefit
from timing and finishing late requests). Maybe in the future some
conditions might be refined but it's harmless.
What happens to such requests is that they're dequeued and their pendconn
freed, so that the caller can decide to try to LB or queue them again. For
now the function is not used, it's just added separately for easier tracking.
(cherry picked from commit 1a8f3a368f1d212f5c2869d400fb07c78b2e7f45)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 94f85bd64609c52f105e95c8fc6500cdc814d642)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 7a30a2d21dfb79eedbbda52754d6486ed4f5a067)
Signed-off-by: Willy Tarreau <w@1wt.eu>
diff --git a/include/haproxy/queue.h b/include/haproxy/queue.h
index e77370c..e4201fb 100644
--- a/include/haproxy/queue.h
+++ b/include/haproxy/queue.h
@@ -39,6 +39,7 @@
int pendconn_redistribute(struct server *s);
int pendconn_grab_from_px(struct server *s);
void pendconn_unlink(struct pendconn *p);
+int pendconn_must_try_again(struct pendconn *p);
/* Removes the pendconn from the server/proxy queue. It supports being called
* with NULL for pendconn and with a pendconn not in the list. It is the
diff --git a/src/queue.c b/src/queue.c
index f20285b..7df3379 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -621,6 +621,68 @@
return 0;
}
+/* checks after a successful pendconn_add() if the connection ended up being
+ * alone with no active connection left to dequeue it. In such a case it will
+ * simply remove it from the queue, free it and return non-zero to inform the
+ * caller that it must try to add the connection again, otherwise it returns
+ * zero, indicating that the connection will be handled normally. The caller
+ * might have to drop SF_DIRECT and/or SF_ASSIGNED if the conn was on a proxy.
+ */
+int pendconn_must_try_again(struct pendconn *p)
+{
+ struct queue *q = p->queue;
+ struct proxy *px = q->px;
+ struct server *sv = q->sv;
+ int ret = 0;
+
+ if (likely(!HA_ATOMIC_LOAD(&p->node.node.leaf_p)))
+ goto leave;
+
+ /* for a server, we need at least one conn left on this server to
+ * find ours.
+ */
+ if (likely(sv && HA_ATOMIC_LOAD(&sv->served)))
+ goto leave;
+
+ /* for a backend, we need at least one conn left on any of this
+ * backend's servers to find ours.
+ */
+ if (likely(!sv && HA_ATOMIC_LOAD(&px->served)))
+ goto leave;
+
+ /* OK the situation is not safe anymore, we need to check if we're
+ * still in the queue under a lock.
+ */
+ HA_SPIN_LOCK(QUEUE_LOCK, &q->lock);
+ HA_SPIN_LOCK(QUEUE_LOCK, &p->del_lock);
+
+ if (p->node.node.leaf_p) {
+ eb32_delete(&p->node);
+ _HA_ATOMIC_DEC(&q->length);
+ _HA_ATOMIC_INC(&q->idx);
+ _HA_ATOMIC_DEC(&px->totpend);
+ ret = 1;
+ }
+
+ HA_SPIN_UNLOCK(QUEUE_LOCK, &p->del_lock);
+ HA_SPIN_UNLOCK(QUEUE_LOCK, &q->lock);
+
+ /* check if the connection was still queued. If not, it means its
+ * processing has begun so it's safe.
+ */
+ if (!ret)
+ goto leave;
+
+ /* The pendconn is not queued anymore and will not be so we're safe
+ * to free it.
+ */
+ p->strm->pend_pos = NULL;
+ pool_free(pool_head_pendconn, p);
+
+leave:
+ return ret;
+}
+
static enum act_return action_set_priority_class(struct act_rule *rule, struct proxy *px,
struct session *sess, struct stream *s, int flags)
{