From 4c1eeb403d64c49fdcf8b1b3cc7a4e9bcd07a3a6 Mon Sep 17 00:00:00 2001 From: Alexey Kopytov Date: Tue, 23 Dec 2014 14:42:54 +0300 Subject: [PATCH] Bug #1405076: Deadlock in RELEASE_LOCK() The RELEASE_LOCK() implementation introduced by the multiple user level locks patch can result in a deadlock under the following conditions: - connection #1 calls RELEASE_LOCK() for a previously acquired lock. In which case MDL_lock::remove_ticket() is called, which write-locks MDL_lock::m_rwlock corresponding to the user-level MDL object. With that lock held, MDL_map_partition::remove() is called which locks MDL_map_partition::m_mutex protecting the hash of MDL locks belonging to an MDL partition. - connection #2 calls RELEASE_LOCK() simultaneously for the same lock being released by connection #1. Since connection #2 did not own the lock, it calls MDL_map_partition::get_lock_owner() to check if 0 or NULL should be returned (i.e. if the lock exists). That function also locks both MDL_map_partition::m_mutex and MDL_lock::m_rwlock(), but in the reverse order as compared to connection #1. With the right timing for the above events we get each thread waiting for a lock acquired by the other thread, i.e. a deadlock. Fixed by avoiding to lock MDL_lock::m_rwlock with MDL_map_partition::m_mutex locked. There is already infrastructure to release the latter and acquire the former and guarantee that a reference to an MDL_lock object is valid at the same time. It is implemented in MDL_map_partition::move_from_hash_to_lock_mutex(), so the fix utilizes it to remove the deadlock condition. --- mysql-test/r/percona_bug1405076.result | 23 +++++++++++ mysql-test/t/percona_bug1405076.test | 54 ++++++++++++++++++++++++++ sql/mdl.cc | 19 ++++++++- 3 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 mysql-test/r/percona_bug1405076.result create mode 100644 mysql-test/t/percona_bug1405076.test diff --git a/mysql-test/r/percona_bug1405076.result b/mysql-test/r/percona_bug1405076.result new file mode 100644 index 000000000000..a1203219a48e --- /dev/null +++ b/mysql-test/r/percona_bug1405076.result @@ -0,0 +1,23 @@ +connection con1 +SELECT GET_LOCK('foo', -1); +GET_LOCK('foo', -1) +1 +SET debug_sync='mdl_lock_remove_ticket_m_rwlock_locked SIGNAL con1_blocked WAIT_FOR go NO_CLEAR_EVENT'; +# sending SELECT RELEASE_LOCK('foo') +SELECT RELEASE_LOCK('foo'); +connection con2 +SET debug_sync='mdl_map_partition_get_lock_owner_m_mutex_locked SIGNAL con2_blocked WAIT_FOR go NO_CLEAR_EVENT'; +# sending SELECT RELEASE_LOCK('foo') +SELECT RELEASE_LOCK('foo');; +connection default +SET debug_sync='now WAIT_FOR con1_blocked'; +SET debug_sync='now WAIT_FOR con2_blocked'; +SET debug_sync='now SIGNAL go'; +connection con1 +RELEASE_LOCK('foo') +1 +connection con2 +RELEASE_LOCK('foo') +NULL +connection default +SET debug_sync='RESET'; diff --git a/mysql-test/t/percona_bug1405076.test b/mysql-test/t/percona_bug1405076.test new file mode 100644 index 000000000000..6ff098ef824a --- /dev/null +++ b/mysql-test/t/percona_bug1405076.test @@ -0,0 +1,54 @@ +######################################################################## +# Bug #1405076: Deadlock in RELEASE_LOCK() +######################################################################## + +--source include/have_debug_sync.inc + +--source include/count_sessions.inc + +--connect (con1, localhost, root,,) +--connect (con2, localhost, root,,) + +--connection con1 +--echo connection con1 + +SELECT GET_LOCK('foo', -1); + +SET debug_sync='mdl_lock_remove_ticket_m_rwlock_locked SIGNAL con1_blocked WAIT_FOR go NO_CLEAR_EVENT'; +--echo # sending SELECT RELEASE_LOCK('foo') +--send SELECT RELEASE_LOCK('foo') + +--connection con2 +--echo connection con2 + +SET debug_sync='mdl_map_partition_get_lock_owner_m_mutex_locked SIGNAL con2_blocked WAIT_FOR go NO_CLEAR_EVENT'; + +--echo # sending SELECT RELEASE_LOCK('foo') +--send SELECT RELEASE_LOCK('foo'); + +--connection default +--echo connection default + +SET debug_sync='now WAIT_FOR con1_blocked'; +SET debug_sync='now WAIT_FOR con2_blocked'; +SET debug_sync='now SIGNAL go'; + +# con1 and con2 would deadlock + +--connection con1 +--echo connection con1 +--reap + +--connection con2 +--echo connection con2 +--reap + +--connection default +--echo connection default + +--disconnect con1 +--disconnect con2 + +SET debug_sync='RESET'; + +--source include/wait_until_count_sessions.inc diff --git a/sql/mdl.cc b/sql/mdl.cc index 83267e3f4f53..97b8159a669e 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -16,6 +16,7 @@ #include "mdl.h" #include "debug_sync.h" +#include "mysqld.h" #include "sql_array.h" #include #include @@ -951,18 +952,29 @@ MDL_map_partition::get_lock_owner(my_hash_value_type hash_value, { unsigned long res = 0; MDL_lock *lock; + +retry: mysql_mutex_lock(&m_mutex); + + DEBUG_SYNC(current_thd, "mdl_map_partition_get_lock_owner_m_mutex_locked"); + lock= (MDL_lock*) my_hash_search_using_hash_value(&m_locks, hash_value, mdl_key->ptr(), mdl_key->length()); if (lock) { - mysql_prlock_rdlock(&lock->m_rwlock); + if (move_from_hash_to_lock_mutex(lock)) + goto retry; + res= lock->get_lock_owner(); mysql_prlock_unlock(&lock->m_rwlock); } - mysql_mutex_unlock(&m_mutex); + else + { + mysql_mutex_unlock(&m_mutex); + } + return res; } @@ -1888,6 +1900,9 @@ MDL_lock::get_lock_owner() const void MDL_lock::remove_ticket(Ticket_list MDL_lock::*list, MDL_ticket *ticket) { mysql_prlock_wrlock(&m_rwlock); + + DEBUG_SYNC(current_thd, "mdl_lock_remove_ticket_m_rwlock_locked"); + (this->*list).remove_ticket(ticket); if (is_empty()) mdl_locks.remove(this);