summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMarko Mäkelä <marko.makela@mariadb.com>2020-12-03 09:11:31 +0200
committerMarko Mäkelä <marko.makela@mariadb.com>2020-12-03 09:11:31 +0200
commit260161fc9fb5b4885013d550e606681769b52019 (patch)
tree3aeb1cb3a4232073f364cd5a47fa6d5c7f0b54f6
parenta13fac9eeef0f304b6b6f52ad2b6659f22190523 (diff)
downloadmariadb-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.h12
-rw-r--r--storage/innobase/sync/srw_lock.cc58
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(); }