From: Mathieu Desnoyers Date: Sun, 8 Dec 2013 15:31:04 +0000 (-0500) Subject: Fix: urcu-bp interaction with threads vs constructors/destructors X-Git-Tag: v0.8.2~15 X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=c2c36f693a047428c17249482a9c42860a41ff09;p=userspace-rcu.git Fix: urcu-bp interaction with threads vs constructors/destructors Add a reference counter for threads using urcu-bp, thus ensuring that even if the urcu destructor is executed before each thread using RCU read-side critical sections exit, those threads will not see a corrupted thread list. Also, don't use URCU_TLS() within urcu_bp_thread_exit_notifier(). It appears that this is racy (although this was probably due to the issue fixed by reference counting). Anyway, play safe, and pass the rcu_key received as parameter instead. Those issues only reproduce when threads are still active when the urcu-bp destructor is called. Signed-off-by: Mathieu Desnoyers --- diff --git a/urcu-bp.c b/urcu-bp.c index fb5ca6c..836ef92 100644 --- a/urcu-bp.c +++ b/urcu-bp.c @@ -91,6 +91,9 @@ void *mremap_wrapper(void *old_address, size_t old_size, */ #define RCU_QS_ACTIVE_ATTEMPTS 100 +static +int rcu_bp_refcount; + static void __attribute__((constructor)) rcu_bp_init(void); static @@ -454,11 +457,8 @@ struct registry_chunk *find_chunk(struct rcu_reader *rcu_reader_reg) /* Called with signals off and mutex locked */ static -void remove_thread(void) +void remove_thread(struct rcu_reader *rcu_reader_reg) { - struct rcu_reader *rcu_reader_reg; - - rcu_reader_reg = URCU_TLS(rcu_reader); cleanup_thread(find_chunk(rcu_reader_reg), rcu_reader_reg); URCU_TLS(rcu_reader) = NULL; } @@ -499,7 +499,7 @@ end: /* Disable signals, take mutex, remove from registry */ static -void rcu_bp_unregister(void) +void rcu_bp_unregister(struct rcu_reader *rcu_reader_reg) { sigset_t newmask, oldmask; int ret; @@ -512,11 +512,12 @@ void rcu_bp_unregister(void) abort(); mutex_lock(&rcu_gp_lock); - remove_thread(); + remove_thread(rcu_reader_reg); mutex_unlock(&rcu_gp_lock); ret = pthread_sigmask(SIG_SETMASK, &oldmask, NULL); if (ret) abort(); + rcu_bp_exit(); } /* @@ -526,15 +527,14 @@ void rcu_bp_unregister(void) static void urcu_bp_thread_exit_notifier(void *rcu_key) { - assert(rcu_key == URCU_TLS(rcu_reader)); - rcu_bp_unregister(); + rcu_bp_unregister(rcu_key); } static void rcu_bp_init(void) { mutex_lock(&init_lock); - if (!initialized) { + if (!rcu_bp_refcount++) { int ret; ret = pthread_key_create(&urcu_bp_key, @@ -549,16 +549,21 @@ void rcu_bp_init(void) static void rcu_bp_exit(void) { - struct registry_chunk *chunk, *tmp; - int ret; + mutex_lock(&init_lock); + if (!--rcu_bp_refcount) { + struct registry_chunk *chunk, *tmp; + int ret; - cds_list_for_each_entry_safe(chunk, tmp, - ®istry_arena.chunk_list, node) { - munmap(chunk, chunk->data_len + sizeof(struct registry_chunk)); + cds_list_for_each_entry_safe(chunk, tmp, + ®istry_arena.chunk_list, node) { + munmap(chunk, chunk->data_len + + sizeof(struct registry_chunk)); + } + ret = pthread_key_delete(urcu_bp_key); + if (ret) + abort(); } - ret = pthread_key_delete(urcu_bp_key); - if (ret) - abort(); + mutex_unlock(&init_lock); } /*