MEDIUM: threads: replace ha_set_tid() with ha_set_thread()

ha_set_tid() was randomly used either to explicitly set thread 0 or to
set any possibly incomplete thread during boot. Let's replace it with
a pointer to a valid thread or NULL for any thread. This allows us to
check that the designated threads are always valid, and to ignore the
thread 0's mapping when setting it to NULL, and always use group 0 with
it during boot.

The initialization code is also cleaner, as we don't pass ugly casts
of a thread ID to a pointer anymore.

diff --git a/include/haproxy/thread.h b/include/haproxy/thread.h
index c70108b5443390b961f307a0e0c4e4ed288f79c6..d55c54e08be971ab5817180fbe63c597650db413 100644 (file)
--- a/include/haproxy/thread.h
+++ b/include/haproxy/thread.h
@@ -88,11 +88,21 @@ enum { tid = 0 };
 
 #define ha_sigmask(how, set, oldset)  sigprocmask(how, set, oldset)
 
-static inline void ha_set_tid(unsigned int tid)
+/* Sets the current thread to a valid one described by <thr>, or to any thread
+ * and any group if NULL (e.g. for use during boot where they're not totally
+ * initialized).
+ */
+static inline void ha_set_thread(const struct thread_info *thr)
 {
-       ti = &ha_thread_info[tid];
-       tg = ti->tg ? ti->tg : &ha_tgroup_info[0];
-       th_ctx = &ha_thread_ctx[tid];
+       if (thr) {
+               ti = thr;
+               tg = ti->tg;
+               th_ctx = &ha_thread_ctx[ti->tid];
+       } else {
+               ti = &ha_thread_info[0];
+               tg = &ha_tgroup_info[0];
+               th_ctx = &ha_thread_ctx[0];
+       }
 }
 
 static inline void thread_idle_now()
@@ -203,18 +213,28 @@ extern THREAD_LOCAL unsigned int tid;      /* The thread id */
 
 #define ha_sigmask(how, set, oldset)  pthread_sigmask(how, set, oldset)
 
-/* sets the thread ID, TID bit and thread cfg/ctx pointers for the current
- * thread. Since it may be called during early boot even before threads are
- * initialized, we have to be extra careful about some fields which may still
- * be null. For example tg may be null during a part of the boot.
+/* Sets the current thread to a valid one described by <thr>, or to any thread
+ * and any group if NULL (e.g. for use during boot where they're not totally
+ * initialized).
  */
-static inline void ha_set_tid(unsigned int data)
+static inline void ha_set_thread(const struct thread_info *thr)
 {
-       tid     = data;
-       tid_bit = (1UL << tid);
-       ti      = &ha_thread_info[tid];
-       tg      = ti->tg ? ti->tg : &ha_tgroup_info[0];
-       th_ctx  = &ha_thread_ctx[tid];
+       if (thr) {
+               BUG_ON(!thr->tid_bit);
+               BUG_ON(!thr->tg);
+
+               ti      = thr;
+               tg      = thr->tg;
+               tid     = thr->tid;
+               tid_bit = thr->tid_bit;
+               th_ctx  = &ha_thread_ctx[tid];
+       } else {
+               tid     = 0;
+               tid_bit = 1;
+               ti      = &ha_thread_info[0];
+               tg      = &ha_tgroup_info[0];
+               th_ctx  = &ha_thread_ctx[0];
+       }
 }
 
 /* Marks the thread as idle, which means that not only it's not doing anything

diff --git a/src/haproxy.c b/src/haproxy.c
index c8b5ee7817671fb524a73eb1d9c7160a9f1ddfea..7a4dc790e896e8c76fec71dc090018547b9e3f52 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -253,6 +253,7 @@ unsigned int rlim_fd_max_at_boot = 0;
 /* per-boot randomness */
 unsigned char boot_seed[20];        /* per-boot random seed (160 bits initially) */
 
+/* takes the thread config in argument or NULL for any thread */
 static void *run_thread_poll_loop(void *data);
 
 /* bitfield of a few warnings to emit just once (WARN_*) */
@@ -836,7 +837,7 @@ static void mworker_loop()
                leave */
 
        fork_poller();
-       run_thread_poll_loop(0);
+       run_thread_poll_loop(NULL);
 }
 
 /*
@@ -2697,7 +2698,7 @@ static void *run_thread_poll_loop(void *data)
        __decl_thread(static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER);
        __decl_thread(static pthread_cond_t  init_cond  = PTHREAD_COND_INITIALIZER);
 
-       ha_set_tid((unsigned long)data);
+       ha_set_thread(data);
        set_thread_cpu_affinity();
        clock_set_local_source();
 
@@ -3397,7 +3398,7 @@ int main(int argc, char **argv)
        haproxy_unblock_signals();
 
        /* Finally, start the poll loop for the first thread */
-       run_thread_poll_loop(0);
+       run_thread_poll_loop(&ha_thread_info[0]);
 
        /* wait for all threads to terminate */
        wait_for_threads_completion();

diff --git a/src/hlua.c b/src/hlua.c
index d5d3f599fa3b9abe8f8b99adc4d5f9a624b660b9..b03d8bb4634f8b936c439ec55509834036b0343a 100644 (file)
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -11060,7 +11060,7 @@ static int hlua_load(char **args, int section_type, struct proxy *curpx,
 
        /* loading for global state */
        hlua_state_id = 0;
-       ha_set_tid(0);
+       ha_set_thread(NULL);
        return hlua_load_state(args[1], hlua_states[0], err);
 }
 
@@ -11104,7 +11104,7 @@ static int hlua_load_per_thread(char **args, int section_type, struct proxy *cur
 
        /* loading for thread 1 only */
        hlua_state_id = 1;
-       ha_set_tid(0);
+       ha_set_thread(NULL);
        return hlua_load_state(args[1], hlua_states[1], err);
 }
 
@@ -11311,7 +11311,7 @@ int hlua_post_init()
 
        /* Perform post init of common thread */
        hlua_state_id = 0;
-       ha_set_tid(0);
+       ha_set_thread(&ha_thread_info[0]);
        ret = hlua_post_init_state(hlua_states[hlua_state_id]);
        if (ret == 0)
                return 0;
@@ -11320,7 +11320,7 @@ int hlua_post_init()
        for (hlua_state_id = 2; hlua_state_id < global.nbthread + 1; hlua_state_id++) {
 
                /* set thread context */
-               ha_set_tid(hlua_state_id - 1);
+               ha_set_thread(&ha_thread_info[hlua_state_id - 1]);
 
                /* Init lua state */
                hlua_states[hlua_state_id] = hlua_init_state(hlua_state_id);
@@ -11336,13 +11336,13 @@ int hlua_post_init()
        }
 
        /* Reset thread context */
-       ha_set_tid(0);
+       ha_set_thread(NULL);
 
        /* Execute post init for all states */
        for (hlua_state_id = 1; hlua_state_id < global.nbthread + 1; hlua_state_id++) {
 
                /* set thread context */
-               ha_set_tid(hlua_state_id - 1);
+               ha_set_thread(&ha_thread_info[hlua_state_id - 1]);
 
                /* run post init */
                ret = hlua_post_init_state(hlua_states[hlua_state_id]);
@@ -11351,7 +11351,7 @@ int hlua_post_init()
        }
 
        /* Reset thread context */
-       ha_set_tid(0);
+       ha_set_thread(NULL);
 
        /* control functions registering. Each function must have:
         *  - only the function_ref[0] set positive and all other to -1
@@ -12057,12 +12057,12 @@ void hlua_init(void) {
 
        /* Init state for common/shared lua parts */
        hlua_state_id = 0;
-       ha_set_tid(0);
+       ha_set_thread(NULL);
        hlua_states[0] = hlua_init_state(0);
 
        /* Init state 1 for thread 0. We have at least one thread. */
        hlua_state_id = 1;
-       ha_set_tid(0);
+       ha_set_thread(NULL);
        hlua_states[1] = hlua_init_state(1);
 
        /* Proxy and server configuration initialisation. */

diff --git a/src/thread.c b/src/thread.c
index 2f904deae4974f3d890c7a4c8f37fc14d672b118..ce6a476598ded6321fb289cff8ef29b6bfdc95f0 100644 (file)
--- a/src/thread.c
+++ b/src/thread.c
@@ -217,7 +217,7 @@ void setup_extra_threads(void *(*handler)(void *))
        /* Create nbthread-1 thread. The first thread is the current process */
        ha_pthread[0] = pthread_self();
        for (i = 1; i < global.nbthread; i++)
-               pthread_create(&ha_pthread[i], NULL, handler, (void *)(long)i);
+               pthread_create(&ha_pthread[i], NULL, handler, &ha_thread_info[i]);
 }
 
 /* waits for all threads to terminate. Does nothing when threads are