BUG/MEDIUM: shctx: use at least thread-based locking on USE_PRIVATE_CACHE
Since threads were introduced in 1.8, the USE_PRIVATE_CACHE mode of the
shctx was not updated to use locks. Originally it was meant to disable
sharing between processes, so it removes the lock/unlock instructions.
But with threads enabled, it's not possible to work like this anymore.
It's easy to see that once built with private cache and threads enabled,
sending violent SSL traffic to the the process instantly makes it die.
The HTTP cache is very likely affected as well.
This patch addresses this by falling back to our native spinlocks when
USE_PRIVATE_CACHE is used. In practice we could use them also for other
modes and remove all older implementations, but this patch aims at keeping
the changes very low and easy to backport. A new SHCTX_LOCK label was
added to help with debugging, but OTHER_LOCK might be usable as well
for backports.
An even lighter approach for backports may consist in always declaring
the lock (or reusing "waiters"), and calling pl_take_s() for the lock()
and pl_drop_s() for the unlock() operation. This could even be used in
all modes (process and threads), even when thread support is disabled.
Subsequent patches will further clean up this area.
This patch must be backported to all supported versions since 1.8.
(cherry picked from commit 9e467af804b3dfa05eaa4677644f5ff0c0e0619f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit ca36771425fbced2c81de5259da304dc4fa0254c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
diff --git a/include/haproxy/shctx-t.h b/include/haproxy/shctx-t.h
index 20d2887..533536b 100644
--- a/include/haproxy/shctx-t.h
+++ b/include/haproxy/shctx-t.h
@@ -17,7 +17,8 @@
#if !defined (USE_PRIVATE_CACHE) && defined(USE_PTHREAD_PSHARED)
#include <pthread.h>
#endif
-#include <haproxy/list.h>
+#include <haproxy/api-t.h>
+#include <haproxy/thread-t.h>
#ifndef SHSESS_BLOCK_MIN_SIZE
#define SHSESS_BLOCK_MIN_SIZE 128
@@ -54,6 +55,8 @@
#else
unsigned int waiters;
#endif
+#else
+ __decl_thread(HA_SPINLOCK_T lock); // used when USE_PRIVATE_CACHE=1
#endif
struct list avail; /* list for active and free blocks */
struct list hot; /* list for locked blocks */
diff --git a/include/haproxy/shctx.h b/include/haproxy/shctx.h
index b258f3c..a4f7543 100644
--- a/include/haproxy/shctx.h
+++ b/include/haproxy/shctx.h
@@ -14,7 +14,7 @@
#ifndef __HAPROXY_SHCTX_H
#define __HAPROXY_SHCTX_H
-#include <haproxy/api-t.h>
+#include <haproxy/api.h>
#include <haproxy/list.h>
#include <haproxy/shctx-t.h>
@@ -28,6 +28,8 @@
#include <sys/syscall.h>
#endif
#endif
+#else
+#include <haproxy/thread.h>
#endif
int shctx_init(struct shared_context **orig_shctx,
@@ -47,9 +49,10 @@
/* Lock functions */
#if defined (USE_PRIVATE_CACHE)
+extern int use_shared_mem;
-#define shctx_lock(shctx)
-#define shctx_unlock(shctx)
+#define shctx_lock(shctx) if (use_shared_mem) HA_SPIN_LOCK(SHCTX_LOCK, &shctx->lock)
+#define shctx_unlock(shctx) if (use_shared_mem) HA_SPIN_UNLOCK(SHCTX_LOCK, &shctx->lock)
#elif defined (USE_PTHREAD_PSHARED)
extern int use_shared_mem;
diff --git a/include/haproxy/thread.h b/include/haproxy/thread.h
index 2a32d42..0c63666 100644
--- a/include/haproxy/thread.h
+++ b/include/haproxy/thread.h
@@ -377,6 +377,7 @@
STK_SESS_LOCK,
APPLETS_LOCK,
PEER_LOCK,
+ SHCTX_LOCK,
SSL_LOCK,
SSL_GEN_CERTS_LOCK,
PATREF_LOCK,
@@ -419,6 +420,7 @@
case STK_SESS_LOCK: return "STK_SESS";
case APPLETS_LOCK: return "APPLETS";
case PEER_LOCK: return "PEER";
+ case SHCTX_LOCK: return "SHCTX";
case SSL_LOCK: return "SSL";
case SSL_GEN_CERTS_LOCK: return "SSL_GEN_CERTS";
case PATREF_LOCK: return "PATREF";
diff --git a/src/shctx.c b/src/shctx.c
index f4002a4..3df307d 100644
--- a/src/shctx.c
+++ b/src/shctx.c
@@ -17,12 +17,8 @@
#include <haproxy/list.h>
#include <haproxy/shctx.h>
-#if !defined (USE_PRIVATE_CACHE)
-
int use_shared_mem = 0;
-#endif
-
/*
* Reserve a new row if <first> is null, put it in the hotlist, set the refcount to 1
* or append new blocks to the row with <first> as first block if non null.
@@ -296,11 +292,9 @@
int i;
struct shared_context *shctx;
int ret;
-#ifndef USE_PRIVATE_CACHE
#ifdef USE_PTHREAD_PSHARED
pthread_mutexattr_t attr;
#endif
-#endif
void *cur;
int maptype = MAP_PRIVATE;
@@ -311,10 +305,8 @@
blocksize = (blocksize + sizeof(void *) - 1) & -sizeof(void *);
extra = (extra + sizeof(void *) - 1) & -sizeof(void *);
-#ifndef USE_PRIVATE_CACHE
if (shared)
maptype = MAP_SHARED;
-#endif
shctx = (struct shared_context *)mmap(NULL, sizeof(struct shared_context) + extra + (maxblocks * (sizeof(struct shared_block) + blocksize)),
PROT_READ | PROT_WRITE, maptype | MAP_ANON, -1, 0);
@@ -326,8 +318,8 @@
shctx->nbav = 0;
-#ifndef USE_PRIVATE_CACHE
if (maptype == MAP_SHARED) {
+#ifndef USE_PRIVATE_CACHE
#ifdef USE_PTHREAD_PSHARED
if (pthread_mutexattr_init(&attr)) {
munmap(shctx, sizeof(struct shared_context) + extra + (maxblocks * (sizeof(struct shared_block) + blocksize)));
@@ -354,9 +346,11 @@
#else
shctx->waiters = 0;
#endif
+#else
+ HA_SPIN_INIT(&shctx->lock);
+#endif
use_shared_mem = 1;
}
-#endif
LIST_INIT(&shctx->avail);
LIST_INIT(&shctx->hot);