Fix for bug #50913 "Deadlock between open_and_lock_tables_derived

and MDL".

Concurrent execution of a multi-DELETE statement and ALTER
TABLE statement which affected one of the tables used in
the multi-DELETE sometimes led to deadlock.
Similar deadlocks might have occured when one performed
INSERT/UPDATE/DELETE on a view and concurrently executed
ALTER TABLE for the view's underlying table, or when one
concurrently executed TRUNCATE TABLE for InnoDB table and
ALTER TABLE for the same table.

These deadlocks were caused by a discrepancy between types of
metadata and thr_lock.cc locks acquired by those statements.

What happened was that multi-DELETE/TRUNCATE/DML-through-the-
view statement in the first connection acquired SR lock on a
table, then ALTER TABLE would come in in the second connection
and acquire SNW metadata lock and TL_WRITE_ALLOW_READ
thr_lock.c lock and then would start waiting for the first
connection during lock upgrade. After that the statement in
the first connection would try to acquire TL_WRITE lock on
table and would start waiting for the second connection,
creating a deadlock.

This patch solves this problem by ensuring that we acquire
SW metadata lock in all cases in which we acquiring write
thr_lock.c lock. This guarantees that deadlocks like the
one described above won't occur since all lock conflicts
in such situation are resolved within MDL subsystem.

This patch also adds assert which should guarantee that
such situations won't arise in future.

mysql-test/r/lock_multi.result:
  Added main test for bug #50913 "Deadlock between
  open_and_lock_tables_derived and MDL".
mysql-test/r/mdl_sync.result:
  Added additional coverage for bug #50913 "Deadlock
  between open_and_lock_tables_derived and MDL".
mysql-test/t/lock_multi.test:
  Added main test for bug #50913 "Deadlock between
  open_and_lock_tables_derived and MDL".
mysql-test/t/mdl_sync.test:
  Added additional coverage for bug #50913 "Deadlock
  between open_and_lock_tables_derived and MDL".
sql/lock.cc:
  Added assert that enforces that when we are locking
  a non-temporary table we have an appropriate type of
  metadata lock on this table.
sql/mysql_priv.h:
  Added separate flag for open_tables() to be able specify that
  SH metadata locks on table to be open should be acquired.
  We can no longer use MYSQL_LOCK_IGNORE_FLUSH flag for this
  as in addition to use in I_S implementation it is also used
  for opening system tables. Since in the latter case we also
  acquire thr_lock.c locks using SH metadata lock in it instead
  of SR or SW locks may lead to deadlock.
sql/sql_base.cc:
  When opening tables don't interpret MYSQL_LOCK_IGNORE_FLUSH
  flag as request to acquire SH metadata locks. This flag is
  also used for opening system tables for which we also take
  thr_lock.c locks and thus proper metadata lock to take in
  this case is SR or SW lock (otherwise deadlocks can occur).
  In cases when SH lock is really required (e.g. when tables
  are open by I_S implementation) we rely on that newly
  introduced MYSQL_OPEN_FORCE_SHARED_HIGH_PRIO_MDL flag is
  used.
sql/sql_delete.cc:
  mysql_truncate_by_delete():
    Adjust type of metadata lock to be requested after changing
    type of thr_lock.c lock for table list element from one
    which was set in parser to TL_WRITE.
    This removes discrepancy between types of these locks which
    allowed deadlocks to creep in.
sql/sql_handler.cc:
  When closing table which was open by HANDLER statement clear
  TABLE::open_by_handler flag. This allows to use this flag as
  a reliable indication that TABLE instance was open by HANDLER
  statement in assert which was added to mysql_lock_tables().
sql/sql_parse.cc:
  multi_delete_set_locks_and_link_aux_tables():
    Adjust type of metadata lock to be requested after changing
    type of thr_lock.c lock for table list element from one
    which was set in parser to TL_WRITE.
    This removes discrepancy between types of these locks which
    allowed deadlocks to creep in.
sql/sql_show.cc:
  Use newly introduced MYSQL_OPEN_FORCE_SHARED_HIGH_PRIO_MDL
  flag in order to acquire SH metadata locks when opening tables
  in I_S implementation.
sql/sql_update.cc:
  Added comment explaining why in multi-update after deciding
  that we need weaker thr_lock.c lock on a table we don't
  downgrade metadata lock on it.
sql/sql_view.cc:
  When merging view into main statement adjust type of metadata
  lock to be requested after changing type of thr_lock.c lock
  for table. This removes discrepancy between types of these
  locks which allowed deadlocks to creep in.

1 files changed, 59 insertions, 0 deletions

diff --git a/mysql-test/t/mdl_sync.test b/mysql-test/t/mdl_sync.test
index 4c1ffc934aa..b59af339229 100644
--- a/mysql-test/t/mdl_sync.test
+++ b/mysql-test/t/mdl_sync.test
@@ -3,6 +3,9 @@
 #
 --source include/have_debug_sync.inc
 
+# We need InnoDB tables for some of the tests.
+--source include/have_innodb.inc
+
 # Save the initial number of concurrent sessions.
 --source include/count_sessions.inc
 
@@ -3315,6 +3318,62 @@ SET @@global.general_log= @old_general_log;
 SET @@global.log_output= @old_log_output;
 SET @@session.sql_log_off= @old_sql_log_off;
 
+
+--echo #
+--echo # Additional coverage for bug #50913 "Deadlock between
+--echo # open_and_lock_tables_derived and MDL". The main test
+--echo # case is in lock_multi.test
+--echo #
+--disable_warnings
+drop table if exists t1;
+--enable_warnings
+set debug_sync= 'RESET';
+connect (con50913_1,localhost,root);
+connect (con50913_2,localhost,root);
+connection default;
+create table t1 (i int) engine=InnoDB;
+
+--echo # Switching to connection 'con50913_1'.
+connection con50913_1;
+set debug_sync= 'thr_multi_lock_after_thr_lock SIGNAL parked WAIT_FOR go';
+--echo # Sending:
+--send alter table t1 add column j int
+
+--echo # Switching to connection 'default'.
+connection default;
+--echo # Wait until ALTER TABLE gets blocked on a sync point after
+--echo # acquiring thr_lock.c lock.
+set debug_sync= 'now WAIT_FOR parked';
+--echo # The below statement should wait on MDL lock and not deadlock on
+--echo # thr_lock.c lock.
+--echo # Sending:
+--send truncate table t1
+
+--echo # Switching to connection 'con50913_2'.
+connection con50913_2;
+--echo # Wait until TRUNCATE TABLE is blocked on MDL lock.
+let $wait_condition=
+  select count(*) = 1 from information_schema.processlist
+  where state = "Waiting for table" and info = "truncate table t1";
+--source include/wait_condition.inc
+--echo # Unblock ALTER TABLE.
+set debug_sync= 'now SIGNAL go';
+
+--echo # Switching to connection 'con50913_1'.
+connection con50913_1;
+--echo # Reaping ALTER TABLE.
+--reap
+
+--echo # Switching to connection 'default'.
+connection default;
+--echo # Reaping TRUNCATE TABLE.
+--reap
+disconnect con50913_1;
+disconnect con50913_2;
+set debug_sync= 'RESET';
+drop table t1;
+
+
 # Check that all connections opened by test cases in this file are really
 # gone so execution of other tests won't be affected by their presence.
 --source include/wait_until_count_sessions.inc