BUG/MEDIUM: memory: Add a rwlock before freeing memory.
When using lockless pools, add a new rwlock, flush_pool. read-lock it when
getting memory from the pool, so that concurrenct access are still
authorized, but write-lock it when we're about to free memory, in
pool_flush() and pool_gc().
The problem is, when removing an item from the pool, we unreference it
to get the next one, however, that pointer may have been free'd in the
meanwhile, and that could provoke a crash if the pointer has been unmapped.
It should be OK to use a rwlock, as normal operations will still be able
to access the pool concurrently, and calls to pool_flush() and pool_gc()
should be pretty rare.
This should be backported to 2.1, 2.0 and 1.9.
(cherry picked from commit 04f5fe87d3d3a222b89420f8c1231461f55ebdeb)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit e16ecc580d88e855994b4676d7c4b18d5afba84c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
diff --git a/src/memory.c b/src/memory.c
index 07e3428..409b4e5 100644
--- a/src/memory.c
+++ b/src/memory.c
@@ -141,6 +141,8 @@
}
#ifndef CONFIG_HAP_LOCKLESS_POOLS
HA_SPIN_INIT(&pool->lock);
+#else
+ HA_RWLOCK_INIT(&pool->flush_lock);
#endif
}
pool->users++;
@@ -223,6 +225,7 @@
if (!pool)
return;
+ HA_RWLOCK_WRLOCK(POOL_LOCK, &pool->flush_lock);
do {
cmp.free_list = pool->free_list;
cmp.seq = pool->seq;
@@ -230,6 +233,7 @@
new.seq = cmp.seq + 1;
} while (!_HA_ATOMIC_DWCAS(&pool->free_list, &cmp, &new));
__ha_barrier_atomic_store();
+ HA_RWLOCK_WRUNLOCK(POOL_LOCK, &pool->flush_lock);
next = cmp.free_list;
while (next) {
temp = next;
@@ -259,6 +263,7 @@
return;
list_for_each_entry(entry, &pools, list) {
+ HA_RWLOCK_WRLOCK(POOL_LOCK, &entry->flush_lock);
while ((int)((volatile int)entry->allocated - (volatile int)entry->used) > (int)entry->minavail) {
struct pool_free_list cmp, new;
@@ -275,6 +280,7 @@
free(cmp.free_list);
_HA_ATOMIC_SUB(&entry->allocated, 1);
}
+ HA_RWLOCK_WRUNLOCK(POOL_LOCK, &entry->flush_lock);
}
_HA_ATOMIC_STORE(&recurse, 0);
@@ -616,7 +622,7 @@
else
ret = 0;
}
- HA_SPIN_LOCK(OTHER_LOCK, &mem_fail_lock);
+ HA_SPIN_LOCK(POOL_LOCK, &mem_fail_lock);
n = snprintf(&mem_fail_str[mem_fail_cur_idx * MEM_FAIL_MAX_CHAR],
MEM_FAIL_MAX_CHAR - 2,
"%d %.18s %d %d", mem_fail_cur_idx, pool->name, ret, tid);
@@ -629,7 +635,7 @@
mem_fail_cur_idx++;
if (mem_fail_cur_idx == MEM_FAIL_MAX_STR)
mem_fail_cur_idx = 0;
- HA_SPIN_UNLOCK(OTHER_LOCK, &mem_fail_lock);
+ HA_SPIN_UNLOCK(POOL_LOCK, &mem_fail_lock);
return ret;
}