Skip to content

Commit

Permalink
Bug #1405076: Deadlock in RELEASE_LOCK()
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Alexey Kopytov committed Dec 23, 2014
1 parent 7e8b64c commit 4c1eeb4
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 2 deletions.
23 changes: 23 additions & 0 deletions mysql-test/r/percona_bug1405076.result
Original file line number Diff line number Diff line change
@@ -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';
54 changes: 54 additions & 0 deletions mysql-test/t/percona_bug1405076.test
Original file line number Diff line number Diff line change
@@ -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
19 changes: 17 additions & 2 deletions sql/mdl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include "mdl.h"
#include "debug_sync.h"
#include "mysqld.h"
#include "sql_array.h"
#include <hash.h>
#include <mysqld_error.h>
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 4c1eeb4

Please sign in to comment.