diff options
author | Marko Mäkelä <marko.makela@mariadb.com> | 2020-12-03 09:11:31 +0200 |
---|---|---|
committer | Marko Mäkelä <marko.makela@mariadb.com> | 2020-12-03 09:11:31 +0200 |
commit | 260161fc9fb5b4885013d550e606681769b52019 (patch) | |
tree | 3aeb1cb3a4232073f364cd5a47fa6d5c7f0b54f6 | |
parent | a13fac9eeef0f304b6b6f52ad2b6659f22190523 (diff) | |
download | mariadb-git-260161fc9fb5b4885013d550e606681769b52019.tar.gz |
MDEV-24167 fixup: Avoid hangs in SRW_LOCK_DUMMY
In commit 1fdc161d8faeb18acf0ccea9b33ad64f0b596f79 we introduced
a mutex-and-condition-variable based fallback implementation
for platforms that lack a futex system call. That implementation
is prone to hangs.
Let us use separate condition variables for shared and exclusive requests.
-rw-r--r-- | storage/innobase/include/srw_lock.h | 12 | ||||
-rw-r--r-- | storage/innobase/sync/srw_lock.cc | 58 |
2 files changed, 45 insertions, 25 deletions
diff --git a/storage/innobase/include/srw_lock.h b/storage/innobase/include/srw_lock.h index 405cffa651b..f305a4de124 100644 --- a/storage/innobase/include/srw_lock.h +++ b/storage/innobase/include/srw_lock.h @@ -35,7 +35,8 @@ class srw_lock_low final : private rw_lock #endif #ifdef SRW_LOCK_DUMMY pthread_mutex_t mutex; - pthread_cond_t cond; + pthread_cond_t cond_shared; + pthread_cond_t cond_exclusive; #endif /** @return pointer to the lock word */ rw_lock *word() { return static_cast<rw_lock*>(this); } @@ -46,11 +47,14 @@ class srw_lock_low final : private rw_lock void write_lock(); /** Wait for signal @param l lock word from a failed acquisition */ - inline void wait(uint32_t l); + inline void writer_wait(uint32_t l); + /** Wait for signal + @param l lock word from a failed acquisition */ + inline void readers_wait(uint32_t l); /** Send signal to one waiter */ - inline void wake_one(); + inline void writer_wake(); /** Send signal to all waiters */ - inline void wake_all(); + inline void readers_wake(); public: #ifdef SRW_LOCK_DUMMY void init(); diff --git a/storage/innobase/sync/srw_lock.cc b/storage/innobase/sync/srw_lock.cc index 1ee5565b9c8..af4af84f95c 100644 --- a/storage/innobase/sync/srw_lock.cc +++ b/storage/innobase/sync/srw_lock.cc @@ -24,48 +24,60 @@ void srw_lock_low::init() { DBUG_ASSERT(!is_locked_or_waiting()); pthread_mutex_init(&mutex, nullptr); - pthread_cond_init(&cond, nullptr); + pthread_cond_init(&cond_shared, nullptr); + pthread_cond_init(&cond_exclusive, nullptr); } void srw_lock_low::destroy() { DBUG_ASSERT(!is_locked_or_waiting()); pthread_mutex_destroy(&mutex); - pthread_cond_destroy(&cond); + pthread_cond_destroy(&cond_shared); + pthread_cond_destroy(&cond_exclusive); } -inline void srw_lock_low::wait(uint32_t l) +inline void srw_lock_low::writer_wait(uint32_t l) { pthread_mutex_lock(&mutex); if (value() == l) - pthread_cond_wait(&cond, &mutex); + pthread_cond_wait(&cond_exclusive, &mutex); pthread_mutex_unlock(&mutex); } -inline void srw_lock_low::wake_one() +inline void srw_lock_low::readers_wait(uint32_t l) { pthread_mutex_lock(&mutex); - pthread_cond_signal(&cond); + if (value() == l) + pthread_cond_wait(&cond_shared, &mutex); pthread_mutex_unlock(&mutex); } -inline void srw_lock_low::wake_all() +inline void srw_lock_low::writer_wake() { pthread_mutex_lock(&mutex); - pthread_cond_broadcast(&cond); + uint32_t l= value(); + if (l & WRITER) + DBUG_ASSERT(!(l & ~WRITER_PENDING)); + else + { + pthread_cond_broadcast(&cond_exclusive); + if (!(l & WRITER_PENDING)) + pthread_cond_broadcast(&cond_shared); + } pthread_mutex_unlock(&mutex); } +# define readers_wake writer_wake #else static_assert(4 == sizeof(rw_lock), "ABI"); # ifdef _WIN32 # include <synchapi.h> -inline void srw_lock_low::wait(uint32_t l) +inline void srw_lock_low::writer_wait(uint32_t l) { WaitOnAddress(word(), &l, 4, INFINITE); } -inline void srw_lock_low::wake_one() { WakeByAddressSingle(word()); } -inline void srw_lock_low::wake_all() { WakeByAddressAll(word()); } +inline void srw_lock_low::writer_wake() { WakeByAddressSingle(word()); } +inline void srw_lock_low::readers_wake() { WakeByAddressAll(word()); } # else # ifdef __linux__ # include <linux/futex.h> @@ -81,10 +93,14 @@ inline void srw_lock_low::wake_all() { WakeByAddressAll(word()); } # error "no futex support" # endif -inline void srw_lock_low::wait(uint32_t l) { SRW_FUTEX(word(), WAIT, l); } -inline void srw_lock_low::wake_one() { SRW_FUTEX(word(), WAKE, 1); } -inline void srw_lock_low::wake_all() { SRW_FUTEX(word(), WAKE, INT_MAX); } +inline void srw_lock_low::writer_wait(uint32_t l) +{ + SRW_FUTEX(word(), WAIT, l); +} +inline void srw_lock_low::writer_wake() { SRW_FUTEX(word(), WAKE, 1); } +inline void srw_lock_low::readers_wake() { SRW_FUTEX(word(), WAKE, INT_MAX); } # endif +# define readers_wait writer_wait #endif /** Wait for a read lock. @@ -99,15 +115,15 @@ void srw_lock_low::read_lock(uint32_t l) #ifdef SRW_LOCK_DUMMY pthread_mutex_lock(&mutex); { - pthread_cond_signal(&cond); - pthread_cond_wait(&cond, &mutex); + pthread_cond_signal(&cond_exclusive); + pthread_cond_wait(&cond_shared, &mutex); l= value(); } while (l == WRITER_WAITING); pthread_mutex_unlock(&mutex); continue; #else - wake_one(); + writer_wake(); #endif } else @@ -120,7 +136,7 @@ void srw_lock_low::read_lock(uint32_t l) goto wake_writer; } - wait(l); + readers_wait(l); } while (!read_trylock(l)); } @@ -153,10 +169,10 @@ void srw_lock_low::write_lock() else DBUG_ASSERT(~WRITER_WAITING & l); - wait(l); + writer_wait(l); } } -void srw_lock_low::rd_unlock() { if (read_unlock()) wake_one(); } +void srw_lock_low::rd_unlock() { if (read_unlock()) writer_wake(); } -void srw_lock_low::wr_unlock() { write_unlock(); wake_all(); } +void srw_lock_low::wr_unlock() { write_unlock(); readers_wake(); } |