diff --git a/.github/workflows/ci-linux.yml b/.github/workflows/ci-linux.yml index 892ce9788e..5bd18fdc92 100644 --- a/.github/workflows/ci-linux.yml +++ b/.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 diff --git a/src/bthread/mutex.cpp b/src/bthread/mutex.cpp index 403f6bb8a8..9168dc34c1 100644 --- a/src/bthread/mutex.cpp +++ b/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*)&owner->type) + ->store(OWNER_TYPE_BTHREAD, butil::memory_order_release); + } else { + owner->id = pthread_numeric_id(); + ((butil::atomic*)&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*)&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,6 +877,9 @@ void FastPthreadMutex::lock() { if (split->locked.exchange(1, butil::memory_order_acquire)) { (void)lock_contended(); } + _owner.id = pthread_numeric_id(); + ((butil::atomic*)&_owner.type) + ->store(OWNER_TYPE_PTHREAD,butil::memory_order_release); ++tls_pthread_lock_count; } @@ -842,12 +887,16 @@ 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*)&_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*)&_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*)&_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*)&m->owner.type) + ->store(OWNER_TYPE_NONE,butil::memory_order_release); m->butex = bthread::butex_create_checked(); if (!m->butex) { return ENOMEM; @@ -889,11 +951,7 @@ 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) { @@ -901,8 +959,7 @@ int bthread_mutex_lock_contended(bthread_mutex_t* m) { } 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) { diff --git a/src/bthread/mutex.h b/src/bthread/mutex.h index ad6d2e5cbd..15f579958b 100644 --- a/src/bthread/mutex.h +++ b/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; diff --git a/src/bthread/types.h b/src/bthread/types.h index 4b4f0565f5..bbec9af94b 100644 --- a/src/bthread/types.h +++ b/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 { diff --git a/test/bthread_mutex_unittest.cpp b/test/bthread_mutex_unittest.cpp index 21bd60446f..e62566c271 100644 --- a/test/bthread_mutex_unittest.cpp +++ b/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); }