MySQL Bugs: #46224: HANDLER statements within a transaction might lead to deadlocks

Bug #46224	HANDLER statements within a transaction might lead to deadlocks
Submitted:	16 Jul 2009 14:08	Modified:	7 Mar 2010 2:07
Reporter:	Davi Arnaut (OCA)	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: Locking	Severity:	S2 (Serious)
Version:	5.4+	OS:	Any
Assigned to:	Konstantin Osipov	CPU Architecture:	Any

Description:
The problem is that issuing HANDLER statements within a transaction that already holds metadata locks might lead to deadlocks. This can happen because HANDLER statements fail to properly detect a deadlock scenario while waiting for a table -- a session should never wait for a table if it already holds metadata locks.

Also not that this test case also triggers a problem in the process of upgrading a shared metadata lock to a exclusive one. The handler statement shouldn't be able acquire the shared metadata lock while there is a pending exclusive lock on the table.

How to repeat:
CREATE TABLE t1 (a INT, KEY a(a));

connect(con1,localhost,root,,);
connect(con2,localhost,root,,);
connect(con3,localhost,root,,);

connection con1;
BEGIN;
SELECT * FROM t1;
HANDLER t1 OPEN;
connection con2;
LOCK TABLES t1 WRITE;
connection con1;
--send HANDLER t1 READ a NEXT;
connection con2;
let $wait_condition=
  SELECT COUNT(*) = 1 FROM INFORMATION_SCHEMA.PROCESSLIST
  WHERE STATE = "Table lock" AND INFO = "HANDLER t1 READ a NEXT";
--source include/wait_condition.inc
--send DROP TABLE t1;
connection con3;
let $wait_condition=
  SELECT COUNT(*) = 1 FROM INFORMATION_SCHEMA.PROCESSLIST
  WHERE STATE = "Waiting for table" AND INFO = "DROP TABLE t1";
--source include/wait_condition.inc
connection con1;
--reap

Suggested fix:
Disable HANDLER statements within transactions or make HANDLER statements aware of a transactional environment.
Ensure that shared metadata locks aren't granted if there is a pending exclusive lock on the table due to a shared metadata lock being upgraded.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/87623

3659 Jon Olav Hauglid	2009-10-21
      Bug #46224 HANDLER statements within a transaction might lead to deadlocks
      
      The the problem was that using HANDLER statements within a transaction
      that already holds metadata locks might lead to deadlocks. The handler
      code failed to properly detect deadlock situations and report those
      as ER_LOCK_DEADLOCK.
      
      The reason was that HANDLER statements would retry indefinitly in
      cases where it needed to reopen the handler - even if inside a 
      transaction that was already holding locks. This patch changes the
      reopen code to report ER_LOCK_DEADLOCK in such situations.
      
      Testcase based on the bug description added to handler_myisam.test.
     @ sql/mdl.cc
        With the removal of backup_mdl_context in sql_handler.cc, this
        is no longer true if FLUSH TABLE WITH READ LOCK is executed
        with open HANDLERs. See mysql-test/include/handler.inc:702.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/89868

3699 Konstantin Osipov	2009-11-10
      A prerequisite patch for the fix for Bug#46224 
      "HANDLER statements within a transaction might lead to deadlocks".
      Introduce a notion of a sentinel to MDL_context. A sentinel
      is a ticket that separates all tickets in the context into two
      groups: before and after it. Currently we can have (and need) only
      one designated sentinel -- it separates all locks taken by LOCK
      TABLE or HANDLER statement, which must survive COMMIT and ROLLBACK
      and all other locks, which must be released at COMMIT and ROLLBACK.
      The tricky part is maintaining the sentinel up to date when
      someone release its corresponding ticket. This can happen, e.g.
      if someone issues DROP TABLE under LOCK TABLES (generally,
      see all calls to release_all_locks_for_name()). 
      MDL_context::release_ticket() is modified to take care for it.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/89872

3699 Konstantin Osipov	2009-11-10
      A prerequisite patch for the fix for Bug#46224 
      "HANDLER statements within a transaction might lead to deadlocks".
      Introduce a notion of a sentinel to MDL_context. A sentinel
      is a ticket that separates all tickets in the context into two
      groups: before and after it. Currently we can have (and need) only
      one designated sentinel -- it separates all locks taken by LOCK
      TABLE or HANDLER statement, which must survive COMMIT and ROLLBACK
      and all other locks, which must be released at COMMIT and ROLLBACK.
      The tricky part is maintaining the sentinel up to date when
      someone release its corresponding ticket. This can happen, e.g.
      if someone issues DROP TABLE under LOCK TABLES (generally,
      see all calls to release_all_locks_for_name()). 
      MDL_context::release_ticket() is modified to take care of it.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/89873

3699 Konstantin Osipov	2009-11-10
      A prerequisite patch for the fix for Bug#46224 
      "HANDLER statements within a transaction might lead to deadlocks".
      Introduce a notion of a sentinel to MDL_context. A sentinel
      is a ticket that separates all tickets in the context into two
      groups: before and after it. Currently we can have (and need) only
      one designated sentinel -- it separates all locks taken by LOCK
      TABLE or HANDLER statement, which must survive COMMIT and ROLLBACK
      and all other locks, which must be released at COMMIT and ROLLBACK.
      The tricky part is maintaining the sentinel up to date when
      someone release its corresponding ticket. This can happen, e.g.
      if someone issues DROP TABLE under LOCK TABLES (generally,
      see all calls to release_all_locks_for_name()). 
      MDL_context::release_ticket() is modified to take care of it.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/90120

3700 Konstantin Osipov	2009-11-11
      Intermediate commit for Bug#46224 "HANDLER statements within 
      a transaction might lead to deadlocks"

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/90172

3700 Konstantin Osipov	2009-11-12
      A fix for Bug#46224 "HANDLER statements within 
      a transaction might lead to deadlocks"

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

http://lists.mysql.com/commits/90835

3699 Konstantin Osipov 2009-11-18
A prerequisite patch for the fix for Bug#46224
"HANDLER statements within a transaction might lead to deadlocks".
Introduce a notion of a sentinel to MDL_context. A sentinel
is a ticket that separates all tickets in the context into two
groups: before and after it. Currently we can have (and need) only
one designated sentinel -- it separates all locks taken by LOCK
TABLE or HANDLER statement, which must survive COMMIT and ROLLBACK
and all other locks, which must be released at COMMIT and ROLLBACK.
The tricky part is maintaining the sentinel up to date when
someone release its corresponding ticket. This can happen, e.g.
if someone issues DROP TABLE under LOCK TABLES (generally,
see all calls to release_all_locks_for_name()).
MDL_context::release_ticket() is modified to take care of it.

******
A fix and a test case for Bug#46224 "HANDLER statements within a
transaction might lead to deadlocks".

An attempt to mix HANDLER SQL and transactions could lead to a deadlock.

Incompatible change: entering LOCK TABLES mode automatically
closes all open HANDLERs in the current connection.

Incompatible change: previously an attempt to wait on a lock
in a connection that has an open HANDLER statement could wait
indefinitely/deadlock. After this patch, an error ER_LOCK_DEADLOCK
is produced.

The idea of the fix is to merge thd->handler_mdl_context
with the main mdl_context of the connection, used for transactional
locks. This makes deadlock detection possible, since all waits
with locks are "visible" and available to analysis in a single
MDL context of a connection.

Since HANDLER locks and transactional locks have a different life
cycle -- HANDLERs are explicitly open and closed, and so
are HANDLER locks, explicitly acquired and released, whereas
transactional locks "accumulate" till the end of a transaction
and are released only with COMMIT, ROLLBACK and ROLLBACK TO SAVEPOINT,
a concept of "sentinel" was introduced to MDL_context.
All locks, HANDLER and others, reside in the same linked list.
However, a selected element of the list separates locks with
different life cycle. HANDLER locks always reside at the
end of the list, after the sentinel. Transactional locks are
prepended to the beginning of the list, before the sentinel.
Thus, ROLLBACK, COMMIT or ROLLBACK TO SAVEPOINT, only
release those locks that reside before the sentinel. HANDLER locks
must be released explicitly as part of HANDLER CLOSE statement,
or an implicit close.

@ mysql-test/include/handler.inc
Add test coverage for Bug#46224 "HANDLER statements within a
transaction might lead to deadlocks"
@ mysql-test/r/alter_table-big.result
Make the test pass, it wasn't updated in a while (no
semantical change).
@ mysql-test/r/create-big.result
Make the test pass, it wasn't updated in a while (no
semantical change).
@ mysql-test/r/handler_innodb.result
Update results (Bug#46224).
@ mysql-test/r/handler_myisam.result
Update results (Bug#46224).
@ mysql-test/t/alter_table-big.test
Make the test pass, it wasn't updated in a while (no
semantical change).
@ mysql-test/t/create-big.test
Make the test pass, it wasn't updated in a while (no
semantical change).
@ sql/lock.cc
Remove thd->some_tables_deleted, it's never used.
@ sql/mdl.cc
Implement the concept of HANDLER and LOCK TABLES "sentinel".
Implement a method to clone an acquired ticket.
Do not return tickets beyond the sentinel when acquiring
locks, create a copy.
Remove methods to merge and backup MDL_context, they are now
not used (Hurra!). This opens a path to a proper constructor
and destructor of class MDL_context (to be done in a separate
patch).
Modify find_ticket() to provide information about where
the ticket position is with regard to the sentinel.
@ sql/mdl.h
Add declarations necessary for the implementation of the concept of "sentinel", a dedicated ticket separating transactional
and non-transactional locks.
@ sql/sql_base.cc
Remove thd->some_tables_deleted.
Modify deadlock-prevention asserts and deadlock detection
heuristics to take into account that from now on HANDLER locks
reside in the same locking context.
broadcast_refresh() in mysql_notify_thread_having_shared_lock():
this is necessary if the thread having the shared lock
is asleep in tdc_wait_for_old_versions(). It is only possible
with HANDLER t1 OPEN; FLUSH TABLE (since all over code paths
that lead to tdc_wait_for_old_versions() always have an
empty MDL_context), and previously would deadlock. Now
it's possible to not deadlock in this situation.
@ sql/sql_class.cc
Remove now unused methods. Move mysql_ha_cleanup() a few
lines above in THD::cleanup() to make sure that
all handlers are closed when it's time to destroy the MDL_context
of this connection.
Remove handler_mdl_context and handler_tables.
@ sql/sql_class.h
Remove THD::handler_tables, THD::handler_mdl_context, THD::some_tables_deleted.
@ sql/sql_delete.cc
Remove a duplicate mysql_ha_rm_tables() (bad merge).
@ sql/sql_handler.cc
Remove thd->handler_tables.
Remove thd->handler_mdl_context.
Rewrite mysql_ha_open() to have no special provision for MERGE
tables, now that we don't have to manipulate with thd->handler_tables
it's easy to do.
Remove dead code.
Fix a bug in mysql_ha_flush() when we would always flush
a temporary HANDLER when mysql_ha_flush() is called (actually
mysql_ha_flush() never needs to flush temporary tables).
@ sql/sql_insert.cc
Update a comment, no more some_tables_deleted.
@ sql/sql_parse.cc
Implement an incompatible change: entering LOCK TABLES closes
active HANDLERs, if any.
Now that we have a sentinel, we don't need to check
for thd->locked_tables_mode when releasing metadata locks in
COMMIT/ROLLBACK.
@ sql/sql_plist.h
Add new (now necessary) methods to the list class.
@ sql/sql_prepare.cc
Make sure we don't release HANDLER locks when rollback to a
savepoint, set to not keep locks taken at PREPARE.
@ sql/sql_table.cc
Remove thd->some_tables_deleted.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

http://lists.mysql.com/commits/90873

3716 Konstantin Osipov 2009-11-18
A prerequisite patch for the fix for Bug#46224
"HANDLER statements within a transaction might lead to deadlocks".
Introduce a notion of a sentinel to MDL_context. A sentinel
is a ticket that separates all tickets in the context into two
groups: before and after it. Currently we can have (and need) only
one designated sentinel -- it separates all locks taken by LOCK
TABLE or HANDLER statement, which must survive COMMIT and ROLLBACK
and all other locks, which must be released at COMMIT and ROLLBACK.
The tricky part is maintaining the sentinel up to date when
someone release its corresponding ticket. This can happen, e.g.
if someone issues DROP TABLE under LOCK TABLES (generally,
see all calls to release_all_locks_for_name()).
MDL_context::release_ticket() is modified to take care of it.