bthread: use macros for safer thread-local access on ARM (#3380)

Route accesses to tls_task_group, tls_task_group_nosignal, tls_bls and
several mutex-profiling thread-locals (tls_inside_lock, tls_warn_up,
tls_pthread_lock_count, tls_csites, tls_ever_created_keytable) through
the BAIDU_(GET|SET|GET_PTR)_VOLATILE_THREAD_LOCAL macros instead of
touching the raw __thread variables directly.

On aarch64/clang the compiler can incorrectly cache the address of a
thread_local variable across a suspend point (e.g. when a bthread is
rescheduled onto another worker), so raw accesses can silently read a
stale TaskGroup*/LocalStorage. Going through the accessor macros
everywhere forces a fresh, non-cached load/store on those platforms
while staying a plain variable access elsewhere.

Add bthread::tls_bls_ptr() as the single helper for obtaining the
current LocalStorage*, and update all call sites (bthread.cpp,
butex.cpp, fd.cpp, key.cpp, mutex.cpp, task_control.cpp,
brpc/controller.cpp, test/bthread_unittest.cpp) to use it instead of
declaring their own stale/mistyped extern references to tls_task_group
or tls_bls.

Also hardens borrow_keytable()/return_keytable() in key.cpp: the
KeyTableList pointer obtained under the read lock is no longer reused
after re-acquiring the write lock, since pool->list/pool->destroyed
may change concurrently via bthread_keytable_pool_destroy(); the
thread-local list is drained first before falling back to the pool's
global free list.

Author: altman08

src/brpc/controller.cpp

@@ -51,7 +51,7 @@
 #include "bthread/task_group.h"
 
 namespace bthread {
-extern BAIDU_THREAD_LOCAL TaskGroup* tls_task_group;
+EXTERN_BAIDU_VOLATILE_THREAD_LOCAL(TaskGroup*, tls_task_group);
 }
 
 // This is the only place that both client/server must link, so we put
@@ -714,7 +714,7 @@ void Controller::OnVersionedRPCReturned(const CompletionInfo& info,
             response_attachment().clear();
 
             // Retry backoff.
-            bthread::TaskGroup* g = bthread::tls_task_group;
+            bthread::TaskGroup* g = bthread::BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group);
             int64_t backoff_time_us = retry_policy->GetBackoffTimeMs(this) * 1000L;
             if (backoff_time_us > 0 &&
                 backoff_time_us < _deadline_us - butil::gettimeofday_us()) {

src/brpc/span.h

@@ -36,10 +36,6 @@
 #include "brpc/options.pb.h"                 // ProtocolType
 #include "brpc/span.pb.h"
 
-namespace bthread {
-extern __thread bthread::LocalStorage tls_bls;
-}
-
 namespace brpc {
 
 class Span;

src/bthread/bthread.cpp

@@ -86,7 +86,6 @@ pthread_mutex_t g_task_control_mutex = PTHREAD_MUTEX_INITIALIZER;
 // are not constructed before main().
 TaskControl* g_task_control = NULL;
 
-extern BAIDU_THREAD_LOCAL TaskGroup* tls_task_group;
 EXTERN_BAIDU_VOLATILE_THREAD_LOCAL(TaskGroup*, tls_task_group);
 extern void (*g_worker_startfn)();
 extern void (*g_tagged_worker_startfn)(bthread_tag_t);
@@ -263,7 +262,7 @@ static bool validate_bthread_concurrency_by_tag(const char*, int32_t val) {
     return bthread_setconcurrency_by_tag(val, FLAGS_bthread_current_tag) == 0;
 }
 
-__thread TaskGroup* tls_task_group_nosignal = NULL;
+BAIDU_VOLATILE_THREAD_LOCAL(TaskGroup*, tls_task_group_nosignal, NULL);
 
 BUTIL_FORCE_INLINE int
 start_from_non_worker(bthread_t* __restrict tid,
@@ -283,10 +282,18 @@ start_from_non_worker(bthread_t* __restrict tid,
         // 1. NOSIGNAL is often for creating many bthreads in batch,
         //    inserting into the same TaskGroup maximizes the batch.
         // 2. bthread_flush() needs to know which TaskGroup to flush.
-        TaskGroup* g = tls_task_group_nosignal;
+        auto g = BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group_nosignal);
         if (NULL == g) {
             g = c->choose_one_group(tag);
-            tls_task_group_nosignal = g;
+            BAIDU_SET_VOLATILE_THREAD_LOCAL(tls_task_group_nosignal, g);
+        } else {
+            // tls_task_group_nosignal is not tag-aware: mixing different
+            // tags with BTHREAD_NOSIGNAL on the same non-worker thread is
+            // not supported, otherwise bthreads would be silently inserted
+            // into a TaskGroup of the wrong tag.
+            CHECK_EQ(g->tag(), tag) << "Mixing different tags with "
+                "BTHREAD_NOSIGNAL on the same thread is not supported, "
+                "expected tag=" << g->tag() << " but got tag=" << tag;
         }
         return g->start_background<true>(tid, attr, fn, arg);
     }
@@ -298,7 +305,8 @@ start_from_non_worker(bthread_t* __restrict tid,
 // tag equal to thread local
 // tag equal to BTHREAD_TAG_INVALID
 BUTIL_FORCE_INLINE bool can_run_thread_local(const bthread_attr_t* __restrict attr) {
-    return attr == nullptr || attr->tag == tls_task_group->tag() ||
+    return attr == nullptr ||
+           attr->tag == BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group)->tag() ||
            attr->tag == BTHREAD_TAG_INVALID;
 }
 
@@ -360,10 +368,10 @@ void bthread_flush() {
     if (g) {
         return g->flush_nosignal_tasks();
     }
-    g = bthread::tls_task_group_nosignal;
+    g = bthread::BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group_nosignal);
     if (g) {
         // NOSIGNAL tasks were created in this non-worker.
-        bthread::tls_task_group_nosignal = NULL;
+        bthread::BAIDU_SET_VOLATILE_THREAD_LOCAL(tls_task_group_nosignal, NULL);
         return g->flush_nosignal_tasks_remote();
     }
 }

src/bthread/butex.cpp

@@ -187,7 +187,7 @@ int wait_pthread(ButexPthreadWaiter& pw, const timespec* abstime) {
     }
 }
 
-extern BAIDU_THREAD_LOCAL TaskGroup* tls_task_group;
+EXTERN_BAIDU_VOLATILE_THREAD_LOCAL(TaskGroup*, tls_task_group);
 
 // Returns 0 when no need to unschedule or successfully unscheduled,
 // -1 otherwise.
@@ -280,7 +280,8 @@ void butex_destroy(void* butex) {
 
 // if TaskGroup tls_task_group is belong to tag
 inline bool is_same_tag(bthread_tag_t tag) {
-    return tls_task_group && tls_task_group->tag() == tag;
+    auto g = BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group);
+    return g && g->tag() == tag;
 }
 
 //  nosignal is true & tag is same can return true
@@ -290,7 +291,8 @@ inline bool check_nosignal(bool nosignal, bthread_tag_t tag) {
 
 // if tag is same return tls_task_group else choose one group with tag
 inline TaskGroup* get_task_group(TaskControl* c, bthread_tag_t tag) {
-    return is_same_tag(tag) ? tls_task_group : c->choose_one_group(tag);
+    return is_same_tag(tag) ? BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group)
+                            : c->choose_one_group(tag);
 }
 
 inline void run_in_local_task_group(TaskGroup* g, TaskMeta* next_meta, bool nosignal) {
@@ -320,7 +322,7 @@ int butex_wake(void* arg, bool nosignal) {
     ButexBthreadWaiter* bbw = static_cast<ButexBthreadWaiter*>(front);
     unsleep_if_necessary(bbw, get_global_timer_thread());
     TaskGroup* g = get_task_group(bbw->control, bbw->task_meta->attr.tag);
-    if (g == tls_task_group) {
+    if (g == BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group)) {
         run_in_local_task_group(g, bbw->task_meta, nosignal);
     } else {
         g->ready_to_run_remote(bbw->task_meta, check_nosignal(nosignal, g->tag()));
@@ -384,7 +386,7 @@ int butex_wake_n(void* arg, size_t n, bool nosignal) {
         }
     }
     auto g = get_task_group(next->control, next->task_meta->attr.tag);
-    if (g == tls_task_group) {
+    if (g == BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group)) {
         run_in_local_task_group(g, next->task_meta, nosignal);
     } else {
         g->ready_to_run_remote(next->task_meta, check_nosignal(nosignal, g->tag()));
@@ -488,7 +490,9 @@ int butex_requeue(void* arg, void* arg2) {
     }
     ButexBthreadWaiter* bbw = static_cast<ButexBthreadWaiter*>(front);
     unsleep_if_necessary(bbw, get_global_timer_thread());
-    auto g = is_same_tag(bbw->task_meta->attr.tag) ? tls_task_group : NULL;
+    auto g = is_same_tag(bbw->task_meta->attr.tag)
+                 ? BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group)
+                 : NULL;
     if (g) {
         TaskGroup::exchange(&g, bbw->task_meta);
     } else {
@@ -597,7 +601,7 @@ void wait_for_butex(void* arg) {
     // the two functions. The on-stack ButexBthreadWaiter is safe to use and
     // bw->waiter_state will not change again.
     // unsleep_if_necessary(bw, get_global_timer_thread());
-    tls_task_group->ready_to_run(bw->task_meta);
+    BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group)->ready_to_run(bw->task_meta);
     // FIXME: jump back to original thread is buggy.
 
     // // Value unmatched or waiter is already woken up by TimerThread, jump
@@ -677,7 +681,7 @@ int butex_wait(void* arg, int expected_value, const timespec* abstime, bool prep
         butil::atomic_thread_fence(butil::memory_order_acquire);
         return -1;
     }
-    TaskGroup* g = tls_task_group;
+    TaskGroup* g = BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group);
     if (NULL == g || g->is_current_pthread_task()) {
         return butex_wait_from_pthread(g, b, expected_value, abstime, prepend);
     }

src/bthread/fd.cpp

@@ -44,7 +44,7 @@ extern int pthread_fd_wait(int fd, unsigned events, const timespec* abstime);
 
 namespace bthread {
 
-extern BAIDU_THREAD_LOCAL TaskGroup* tls_task_group;
+EXTERN_BAIDU_VOLATILE_THREAD_LOCAL(TaskGroup*, tls_task_group);
 
 template <typename T, size_t NBLOCK, size_t BLOCK_SIZE>
 class LazyArray {
@@ -446,7 +446,7 @@ int bthread_fd_wait(int fd, unsigned events) {
         errno = EINVAL;
         return -1;
     }
-    bthread::TaskGroup* g = bthread::tls_task_group;
+    bthread::TaskGroup* g = bthread::BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group);
     if (NULL != g && !g->is_current_pthread_task()) {
         return bthread::get_epoll_thread(fd).fd_wait(
             fd, events, NULL);
@@ -463,7 +463,7 @@ int bthread_fd_timedwait(int fd, unsigned events,
         errno = EINVAL;
         return -1;
     }
-    bthread::TaskGroup* g = bthread::tls_task_group;
+    bthread::TaskGroup* g = bthread::BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group);
     if (NULL != g && !g->is_current_pthread_task()) {
         return bthread::get_epoll_thread(fd).fd_wait(
             fd, events, abstime);
@@ -473,7 +473,7 @@ int bthread_fd_timedwait(int fd, unsigned events,
 
 int bthread_connect(int sockfd, const sockaddr* serv_addr,
                     socklen_t addrlen) {
-    bthread::TaskGroup* g = bthread::tls_task_group;
+    bthread::TaskGroup* g = bthread::BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group);
     if (NULL == g || g->is_current_pthread_task()) {
         return ::connect(sockfd, serv_addr, addrlen);
     }