diff options
author | Marko Mäkelä <marko.makela@mariadb.com> | 2020-12-16 17:45:01 +0200 |
---|---|---|
committer | Marko Mäkelä <marko.makela@mariadb.com> | 2020-12-16 17:45:01 +0200 |
commit | 07e4b6b2766df4a4774b2887ce4e593069c60178 (patch) | |
tree | 2562049519bbf6e1278d201e096764305c61bfbd /storage | |
parent | cf2480dd77a45cf56389015a1eabf567846c0658 (diff) | |
download | mariadb-git-07e4b6b2766df4a4774b2887ce4e593069c60178.tar.gz |
MDEV-24167 fixup: Wake up all update_lock() in u_unlock()
It turns out that the hang that was fixed in
commit 43d3dad11468bf45d2098ebca70002c49104f298
for the SRW_LOCK_DUMMY implementation is also possible in the futex
implementation. We have observed hangs of ssux_lock_low::u_unlock()
on Windows where the undesirable value is rw_lock::UPDATER, in the
test mariabackup.xb_compressed_encrypted.
The exact sequence of events to the hang is not known, but
it seems that u_unlock() had better always wake up one thread.
Possibly, the case involves multiple blocked u_unlock().
On a busy server, the hang might be 'rescued' by a subsequent
lock acquisition and release that is executed by another thread.
rw_lock::update_unlock(): Change the return type to void.
ssux_lock_low::u_unlock(): Always invoke readers_wake() [sic],
to wake up any pending update_lock() or write_lock().
On futex implementation, this will wake up all waiters.
On SRW_LOCK_DUMMY, writer_wake() and readers_wake() do the same
thing: wake up one write_lock(), or all update_lock() waiters.
Diffstat (limited to 'storage')
-rw-r--r-- | storage/innobase/include/rw_lock.h | 9 | ||||
-rw-r--r-- | storage/innobase/sync/srw_lock.cc | 7 |
2 files changed, 5 insertions, 11 deletions
diff --git a/storage/innobase/include/rw_lock.h b/storage/innobase/include/rw_lock.h index ac01c28f346..cae71e8f6c7 100644 --- a/storage/innobase/include/rw_lock.h +++ b/storage/innobase/include/rw_lock.h @@ -132,14 +132,13 @@ public: DBUG_ASSERT(!(l & WRITER)); /* no write lock must have existed */ return (~WRITER_PENDING & l) == 1; } - /** Release an update lock. - @return whether any writers may have to be woken up */ - bool update_unlock() + /** Release an update lock */ + void update_unlock() { - auto l= lock.fetch_and(~UPDATER, std::memory_order_release); + IF_DBUG_ASSERT(auto l=,) + lock.fetch_and(~UPDATER, std::memory_order_release); /* the update lock must have existed */ DBUG_ASSERT((l & (WRITER | UPDATER)) == UPDATER); - return !(~(WRITER_PENDING | UPDATER) & l); } /** Release an exclusive lock */ void write_unlock() diff --git a/storage/innobase/sync/srw_lock.cc b/storage/innobase/sync/srw_lock.cc index 17260ebb318..45bf7a29c00 100644 --- a/storage/innobase/sync/srw_lock.cc +++ b/storage/innobase/sync/srw_lock.cc @@ -233,13 +233,8 @@ void ssux_lock_low::rd_unlock() { if (read_unlock()) writer_wake(); } void ssux_lock_low::u_unlock() { -#ifdef SRW_LOCK_DUMMY update_unlock(); - writer_wake(); /* Wake up either write_lock() or update_lock() */ -#else - if (update_unlock()) - writer_wake(); /* Wake up one waiter (hopefully the writer) */ -#endif + readers_wake(); /* Wake up all write_lock(), update_lock() */ } void ssux_lock_low::wr_unlock() { write_unlock(); readers_wake(); } |