BUG/MEDIUM: pool: fix rare risk of deadlock in pool_flush()
As reported by github user @JB0925 in issue #2427, there is a possible
crash in pool_flush(). The problem is that if the free_list is not empty
in the first test, and is empty at the moment the xchg() is performed,
for example because another thread called it in parallel, we place a
POOL_BUSY there that is never removed later, causing the next thread to
wait forever.
This was introduced in 2.5 with commit 2a4523f6f ("BUG/MAJOR: pools: fix
possible race with free() in the lockless variant"). It has probably
very rarely been detected, because:
- pool_flush() is only called when stopping is set
- the function does nothing if global pools are disabled, which is
the case on most modern systems with a fast memory allocator.
It's possible to reproduce it by modifying __task_free() to call
pool_flush() on 1% of the calls instead of only when stopping.
The fix is quite simple, it consists in moving the zeroing of the
entry in the break path after verifying that the entry was not already
busy.
This must be backported wherever commit 2a4523f6f is.
(cherry picked from commit b746af9990d2f7fe10d200026831b6eb10c4953f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 7bd8817e3c253e1090e0c10cc126a08538ecec7c)
[wt: no buckets in 2.8]
Signed-off-by: Willy Tarreau <w@1wt.eu>
diff --git a/src/pool.c b/src/pool.c
index 292b27d..447a24f 100644
--- a/src/pool.c
+++ b/src/pool.c
@@ -702,16 +702,21 @@
* replaces it with a NULL. Then the list can be released.
*/
next = pool->free_list;
- do {
+ while (1) {
while (unlikely(next == POOL_BUSY)) {
__ha_cpu_relax();
next = _HA_ATOMIC_LOAD(&pool->free_list);
}
+
if (next == NULL)
- return;
- } while (unlikely((next = _HA_ATOMIC_XCHG(&pool->free_list, POOL_BUSY)) == POOL_BUSY));
- _HA_ATOMIC_STORE(&pool->free_list, NULL);
- __ha_barrier_atomic_store();
+ break;
+
+ next = _HA_ATOMIC_XCHG(&pool->free_list, POOL_BUSY);
+ if (next != POOL_BUSY) {
+ HA_ATOMIC_STORE(&pool->free_list, NULL);
+ break;
+ }
+ }
while (next) {
temp = next;