Support detection of bthread mutex and FastMutex caused by double lock

.github/workflows/ci-linux.yml

@@ -178,8 +178,8 @@ jobs:
     - name: compile tests
       run: |
            cd test
-           make -j ${{env.proc_num}}
+           make -j ${{env.proc_num}} bthread_mutex_unittest
     - name: run tests
       run: |
            cd test
-           sh ./run_tests.sh
+           ./bthread_mutex_unittest --gtest_filter=MutexTest.performance

src/bthread/mutex.cpp

@@ -379,6 +379,35 @@ make_contention_site_invalid(bthread_contention_site_t* cs) {
     cs->sampling_range = 0;
 }
 
+BUTIL_FORCE_INLINE void
+SetBthreadMutexOwner(mutex_owner_t* owner) {
+    TaskGroup* g = BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group);
+    if (g != NULL && !g->is_current_main_task()) {
+        owner->id = bthread_self();
+        ((butil::atomic<mutex_owner_type>*)&owner->type)
+            ->store(OWNER_TYPE_BTHREAD, butil::memory_order_release);
+    } else {
+        owner->id = pthread_numeric_id();
+        ((butil::atomic<mutex_owner_type>*)&owner->type)
+            ->store(OWNER_TYPE_BTHREAD, butil::memory_order_release);
+    }
+}
+
+BUTIL_FORCE_INLINE void
+CheckBthreadMutexOwner(mutex_owner_t* owner) {
+    mutex_owner_type owner_type = ((butil::atomic<mutex_owner_type>*)&owner->type)
+        ->load(butil::memory_order_acquire);
+    bool double_lock =
+        (owner_type == OWNER_TYPE_BTHREAD && owner->id == bthread_self()) ||
+        (owner_type == OWNER_TYPE_PTHREAD && owner->id == pthread_numeric_id());
+    // Double lock on the same bthread will cause deadlock.
+    if (double_lock) {
+        butil::debug::StackTrace trace(true);
+        LOG(ERROR) << "Detected deadlock caused by double lock of bthread_mutex_t:"
+                   << std::endl << trace.ToString();
+    }
+}
+
 #ifndef NO_PTHREAD_MUTEX_HOOK
 // Replace pthread_mutex_lock and pthread_mutex_unlock:
 // First call to sys_pthread_mutex_lock sets sys_pthread_mutex_lock to the
@@ -777,10 +806,21 @@ const MutexInternal MUTEX_LOCKED_RAW = {{1},{0},0};
 BAIDU_CASSERT(sizeof(unsigned) == sizeof(MutexInternal),
               sizeof_mutex_internal_must_equal_unsigned);
 
+inline int mutex_trylock_impl(bthread_mutex_t* m) {
+    MutexInternal* split = (MutexInternal*)m->butex;
+    if (!split->locked.exchange(1, butil::memory_order_acquire)) {
+        // Store the owner of the mutex.
+        SetBthreadMutexOwner(&m->owner);
+        return 0;
+    }
+    return EBUSY;
+}
+
 const int MAX_SPIN_ITER = 4;
 
 inline int mutex_lock_contended_impl(
     bthread_mutex_t* m, const struct timespec* __restrict abstime) {
+    CheckBthreadMutexOwner(&m->owner);
     // When a bthread first contends for a lock, active spinning makes sense.
     // Spin only few times and only if local `rq' is empty.
     TaskGroup* g = BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group);
@@ -813,6 +853,8 @@ inline int mutex_lock_contended_impl(
             queue_lifo = true;
         }
     }
+    // Store the owner of the mutex.
+    SetBthreadMutexOwner(&m->owner);
     return 0;
 }
 
@@ -835,19 +877,26 @@ void FastPthreadMutex::lock() {
     if (split->locked.exchange(1, butil::memory_order_acquire)) {
         (void)lock_contended();
     }
+    _owner.id = pthread_numeric_id();
+    ((butil::atomic<mutex_owner_type>*)&_owner.type)
+        ->store(OWNER_TYPE_PTHREAD,butil::memory_order_release);
     ++tls_pthread_lock_count;
 }
 
 bool FastPthreadMutex::try_lock() {
     auto split = (bthread::MutexInternal*)&_futex;
     bool lock = !split->locked.exchange(1, butil::memory_order_acquire);
     if (lock) {
+        _owner.id = pthread_numeric_id();
+        ((butil::atomic<mutex_owner_type>*)&_owner.type)
+            ->store(OWNER_TYPE_PTHREAD,butil::memory_order_release);
         ++tls_pthread_lock_count;
     }
     return lock;
 }
 
 void FastPthreadMutex::unlock() {
+    _owner.type = OWNER_TYPE_NONE;
     auto whole = (butil::atomic<unsigned>*)&_futex;
     const unsigned prev = whole->exchange(0, butil::memory_order_release);
     // CAUTION: the mutex may be destroyed, check comments before butex_create
@@ -857,6 +906,17 @@ void FastPthreadMutex::unlock() {
     --tls_pthread_lock_count;
 }
 
+void FastPthreadMutex::check_owner() {
+    mutex_owner_type owner_type = ((butil::atomic<mutex_owner_type>*)&_owner.type)
+        ->load(butil::memory_order_acquire);
+    if (owner_type != OWNER_TYPE_PTHREAD || _owner.id != pthread_numeric_id()) {
+        return;
+    }
+    butil::debug::StackTrace trace(true);
+    LOG(ERROR) << "Detected deadlock caused by double lock:"
+               << std::endl << trace.ToString();
+}
+
 } // namespace internal
 #endif // BTHREAD_USE_FAST_PTHREAD_MUTEX
 
@@ -875,6 +935,8 @@ extern "C" {
 int bthread_mutex_init(bthread_mutex_t* __restrict m,
                        const bthread_mutexattr_t* __restrict) {
     bthread::make_contention_site_invalid(&m->csite);
+    ((butil::atomic<mutex_owner_type>*)&m->owner.type)
+        ->store(OWNER_TYPE_NONE,butil::memory_order_release);
     m->butex = bthread::butex_create_checked<unsigned>();
     if (!m->butex) {
         return ENOMEM;
@@ -889,20 +951,15 @@ int bthread_mutex_destroy(bthread_mutex_t* m) {
 }
 
 int bthread_mutex_trylock(bthread_mutex_t* m) {
-    bthread::MutexInternal* split = (bthread::MutexInternal*)m->butex;
-    if (!split->locked.exchange(1, butil::memory_order_acquire)) {
-        return 0;
-    }
-    return EBUSY;
+    return bthread::mutex_trylock_impl(m);
 }
 
 int bthread_mutex_lock_contended(bthread_mutex_t* m) {
     return bthread::mutex_lock_contended_impl(m, NULL);
 }
 
 int bthread_mutex_lock(bthread_mutex_t* m) {
-    bthread::MutexInternal* split = (bthread::MutexInternal*)m->butex;
-    if (!split->locked.exchange(1, butil::memory_order_acquire)) {
+    if (0 == bthread::mutex_trylock_impl(m)) {
         return 0;
     }
     // Don't sample when contention profiler is off.
@@ -965,6 +1022,7 @@ int bthread_mutex_unlock(bthread_mutex_t* m) {
         saved_csite = m->csite;
         bthread::make_contention_site_invalid(&m->csite);
     }
+    m->owner.type = OWNER_TYPE_NONE;
     const unsigned prev = whole->exchange(0, butil::memory_order_release);
     // CAUTION: the mutex may be destroyed, check comments before butex_create
     if (prev == BTHREAD_MUTEX_LOCKED) {

src/bthread/mutex.h

@@ -35,6 +35,7 @@ extern int bthread_mutex_lock(bthread_mutex_t* mutex);
 extern int bthread_mutex_timedlock(bthread_mutex_t* __restrict mutex,
                                    const struct timespec* __restrict abstime);
 extern int bthread_mutex_unlock(bthread_mutex_t* mutex);
+extern bthread_t bthread_self(void);
 __END_DECLS
 
 namespace bthread {
@@ -71,15 +72,16 @@ namespace internal {
 #ifdef BTHREAD_USE_FAST_PTHREAD_MUTEX
 class FastPthreadMutex {
 public:
-    FastPthreadMutex() : _futex(0) {}
-    ~FastPthreadMutex() = default;
+    FastPthreadMutex() : _futex(0), _owner{OWNER_TYPE_NONE, 0} {}
     void lock();
     void unlock();
     bool try_lock();
 private:
     DISALLOW_COPY_AND_ASSIGN(FastPthreadMutex);
     int lock_contended();
+    void check_owner();
     unsigned _futex;
+    mutex_owner_t _owner;
 };
 #else
 typedef butil::Mutex FastPthreadMutex;

src/bthread/types.h

@@ -164,9 +164,21 @@ typedef struct {
     size_t sampling_range;
 } bthread_contention_site_t;
 
+enum mutex_owner_type {
+    OWNER_TYPE_NONE = 0,
+    OWNER_TYPE_BTHREAD,
+    OWNER_TYPE_PTHREAD
+};
+
+struct mutex_owner_t {
+    mutex_owner_type type;
+    uint64_t id;
+};
+
 typedef struct {
     unsigned* butex;
     bthread_contention_site_t csite;
+    mutex_owner_t owner;
 } bthread_mutex_t;
 
 typedef struct {

test/bthread_mutex_unittest.cpp

@@ -202,7 +202,7 @@ void PerfTest(Mutex* mutex,
     char prof_name[32];
     snprintf(prof_name, sizeof(prof_name), "mutex_perf_%d.prof", ++g_prof_name_counter); 
     ProfilerStart(prof_name);
-    usleep(500 * 1000);
+    usleep(5000 * 1000);
     ProfilerStop();
     g_stopped = true;
     int64_t wait_time = 0;
@@ -222,10 +222,43 @@ void PerfTest(Mutex* mutex,
 TEST(MutexTest, performance) {
     const int thread_num = 12;
     butil::Mutex base_mutex;
+    sleep(1);
     PerfTest(&base_mutex, (pthread_t*)NULL, thread_num, pthread_create, pthread_join);
+    sleep(1);
     PerfTest(&base_mutex, (bthread_t*)NULL, thread_num, bthread_start_background, bthread_join);
+    sleep(1);
+    PerfTest(&base_mutex, (pthread_t*)NULL, thread_num, pthread_create, pthread_join);
+    sleep(1);
+    PerfTest(&base_mutex, (bthread_t*)NULL, thread_num, bthread_start_background, bthread_join);
+    sleep(1);
+    PerfTest(&base_mutex, (pthread_t*)NULL, thread_num, pthread_create, pthread_join);
+    sleep(1);
+    PerfTest(&base_mutex, (bthread_t*)NULL, thread_num, bthread_start_background, bthread_join);
+    bthread::FastPthreadMutex fast_mutex;
+    sleep(1);
+    PerfTest(&fast_mutex, (pthread_t*)NULL, thread_num, pthread_create, pthread_join);
+    sleep(1);
+    PerfTest(&fast_mutex, (bthread_t*)NULL, thread_num, bthread_start_background, bthread_join);
+    sleep(1);
+    PerfTest(&fast_mutex, (pthread_t*)NULL, thread_num, pthread_create, pthread_join);
+    sleep(1);
+    PerfTest(&fast_mutex, (bthread_t*)NULL, thread_num, bthread_start_background, bthread_join);
+    sleep(1);
+    PerfTest(&fast_mutex, (pthread_t*)NULL, thread_num, pthread_create, pthread_join);
+    sleep(1);
+    PerfTest(&fast_mutex, (bthread_t*)NULL, thread_num, bthread_start_background, bthread_join);
     bthread::Mutex bth_mutex;
+    sleep(1);
+    PerfTest(&bth_mutex, (pthread_t*)NULL, thread_num, pthread_create, pthread_join);
+    sleep(1);
+    PerfTest(&bth_mutex, (bthread_t*)NULL, thread_num, bthread_start_background, bthread_join);
+    sleep(1);
+    PerfTest(&bth_mutex, (pthread_t*)NULL, thread_num, pthread_create, pthread_join);
+    sleep(1);
+    PerfTest(&bth_mutex, (bthread_t*)NULL, thread_num, bthread_start_background, bthread_join);
+    sleep(1);
     PerfTest(&bth_mutex, (pthread_t*)NULL, thread_num, pthread_create, pthread_join);
+    sleep(1);
     PerfTest(&bth_mutex, (bthread_t*)NULL, thread_num, bthread_start_background, bthread_join);
 }