diff --git a/mysql-test/r/lock_multi.result b/mysql-test/r/lock_multi.result index 4b08c175ee2..7a5fb445edf 100644 --- a/mysql-test/r/lock_multi.result +++ b/mysql-test/r/lock_multi.result @@ -302,3 +302,49 @@ LOCK TABLES t1 WRITE; FLUSH TABLE t1; DROP TABLE t1; DROP VIEW v1; +# +# Test for bug #50913 "Deadlock between open_and_lock_tables_derived +# and MDL". Also see additional coverage in mdl_sync.test. +# +drop table if exists t1; +drop view if exists v1; +create table t1 (i int); +create view v1 as select i from t1; +begin; +select * from t1; +i +# Switching to connection 'con50913'. +# Sending: +alter table t1 add column j int; +# Switching to connection 'default'. +# Wait until ALTER TABLE gets blocked. +# The below statement should try to acquire SW lock on 't1' +# and therefore should get ER_LOCK_DEADLOCK error. Before +# bug fix it acquired SR lock and hung on thr_lock.c lock. +delete a from t1 as a where i = 1; +ERROR 40001: Deadlock found when trying to get lock; try restarting transaction +# Unblock ALTER TABLE. +commit; +# Switching to connection 'con50913'. +# Reaping ALTER TABLE; +# Switching to connection 'default'. +begin; +select * from v1; +i +# Switching to connection 'con50913'. +# Sending: +alter table t1 drop column j; +# Switching to connection 'default'. +# Wait until ALTER TABLE gets blocked. +# The below statement should try to acquire SW lock on 't1' +# and therefore should get ER_LOCK_DEADLOCK error. Before +# bug fix it acquired SR lock and hung on thr_lock.c lock. +insert into v1 values (1); +ERROR 40001: Deadlock found when trying to get lock; try restarting transaction +# Unblock ALTER TABLE. +commit; +# Switching to connection 'con50913'. +# Reaping ALTER TABLE; +# Switching to connection 'default'. +drop view v1; +drop table t1; diff --git a/mysql-test/r/mdl_sync.result b/mysql-test/r/mdl_sync.result index 1f1211d964e..1e88d2ad27a 100644 --- a/mysql-test/r/mdl_sync.result +++ b/mysql-test/r/mdl_sync.result @@ -2292,3 +2292,33 @@ SET DEBUG_SYNC= 'RESET'; SET @@global.general_log= @old_general_log; SET @@global.log_output= @old_log_output; SET @@session.sql_log_off= @old_sql_log_off; +# +# Additional coverage for bug #50913 "Deadlock between +# open_and_lock_tables_derived and MDL". The main test +# case is in lock_multi.test +# +drop table if exists t1; +set debug_sync= 'RESET'; +create table t1 (i int) engine=InnoDB; +# Switching to connection 'con50913_1'. +set debug_sync= 'thr_multi_lock_after_thr_lock SIGNAL parked WAIT_FOR go'; +# Sending: +alter table t1 add column j int; +# Switching to connection 'default'. +# Wait until ALTER TABLE gets blocked on a sync point after +# acquiring thr_lock.c lock. +set debug_sync= 'now WAIT_FOR parked'; +# The below statement should wait on MDL lock and not deadlock on +# thr_lock.c lock. +# Sending: +truncate table t1; +# Switching to connection 'con50913_2'. +# Wait until TRUNCATE TABLE is blocked on MDL lock. +# Unblock ALTER TABLE. +set debug_sync= 'now SIGNAL go'; +# Switching to connection 'con50913_1'. +# Reaping ALTER TABLE. +# Switching to connection 'default'. +# Reaping TRUNCATE TABLE. +set debug_sync= 'RESET'; +drop table t1; diff --git a/mysql-test/t/lock_multi.test b/mysql-test/t/lock_multi.test index df473876c9a..5b64f60b6eb 100644 --- a/mysql-test/t/lock_multi.test +++ b/mysql-test/t/lock_multi.test @@ -800,5 +800,82 @@ DROP TABLE t1; DROP VIEW v1; +--echo # +--echo # Test for bug #50913 "Deadlock between open_and_lock_tables_derived +--echo # and MDL". Also see additional coverage in mdl_sync.test. +--echo # +--disable_warnings +drop table if exists t1; +drop view if exists v1; +--enable_warnings +connect (con50913,localhost,root); +connection default; +create table t1 (i int); +create view v1 as select i from t1; +begin; +select * from t1; + +--echo # Switching to connection 'con50913'. +connection con50913; +--echo # Sending: +--send alter table t1 add column j int + +--echo # Switching to connection 'default'. +connection default; +--echo # Wait until ALTER TABLE gets blocked. +let $wait_condition= + select count(*) = 1 from information_schema.processlist + where state = "Waiting for table" and info = "alter table t1 add column j int"; +--source include/wait_condition.inc +--echo # The below statement should try to acquire SW lock on 't1' +--echo # and therefore should get ER_LOCK_DEADLOCK error. Before +--echo # bug fix it acquired SR lock and hung on thr_lock.c lock. +--error ER_LOCK_DEADLOCK +delete a from t1 as a where i = 1; +--echo # Unblock ALTER TABLE. +commit; + +--echo # Switching to connection 'con50913'. +connection con50913; +--echo # Reaping ALTER TABLE; +--reap + +--echo # Switching to connection 'default'. +connection default; +begin; +select * from v1; + +--echo # Switching to connection 'con50913'. +connection con50913; +--echo # Sending: +--send alter table t1 drop column j + +--echo # Switching to connection 'default'. +connection default; +--echo # Wait until ALTER TABLE gets blocked. +let $wait_condition= + select count(*) = 1 from information_schema.processlist + where state = "Waiting for table" and info = "alter table t1 drop column j"; +--source include/wait_condition.inc +--echo # The below statement should try to acquire SW lock on 't1' +--echo # and therefore should get ER_LOCK_DEADLOCK error. Before +--echo # bug fix it acquired SR lock and hung on thr_lock.c lock. +--error ER_LOCK_DEADLOCK +insert into v1 values (1); +--echo # Unblock ALTER TABLE. +commit; + +--echo # Switching to connection 'con50913'. +connection con50913; +--echo # Reaping ALTER TABLE; +--reap + +--echo # Switching to connection 'default'. +connection default; +disconnect con50913; +drop view v1; +drop table t1; + + # Wait till all disconnects are completed --source include/wait_until_count_sessions.inc 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 diff --git a/sql/lock.cc b/sql/lock.cc index f34b6c80872..97756893e2b 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -153,6 +153,25 @@ int mysql_lock_tables_check(THD *thd, TABLE **tables, uint count, uint flags) { system_count++; } + + /* + If we are going to lock a non-temporary table we must own metadata + lock of appropriate type on it (I.e. for table to be locked for + write we must own metadata lock of MDL_SHARED_WRITE or stronger + type. For table to be locked for read we must own metadata lock + of MDL_SHARED_READ or stronger type). + The only exception are HANDLER statements which are allowed to + lock table for read while having only MDL_SHARED lock on it. + */ + DBUG_ASSERT(t->s->tmp_table || + thd->mdl_context.is_lock_owner(MDL_key::TABLE, + t->s->db.str, t->s->table_name.str, + t->reginfo.lock_type >= TL_WRITE_ALLOW_WRITE ? + MDL_SHARED_WRITE : MDL_SHARED_READ) || + (t->open_by_handler && + thd->mdl_context.is_lock_owner(MDL_key::TABLE, + t->s->db.str, t->s->table_name.str, + MDL_SHARED))); } /* diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index 5ca70302952..6f207ccb00e 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -2164,6 +2164,11 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **table, uint count, #define MYSQL_OPEN_FAIL_ON_MDL_CONFLICT 0x0200 /** Open tables using MDL_SHARED lock instead of one specified in parser. */ #define MYSQL_OPEN_FORCE_SHARED_MDL 0x0400 +/** + Open tables using MDL_SHARED_HIGH_PRIO lock instead of one specified + in parser. +*/ +#define MYSQL_OPEN_FORCE_SHARED_HIGH_PRIO_MDL 0x0800 /** Please refer to the internals manual. */ #define MYSQL_OPEN_REOPEN (MYSQL_LOCK_IGNORE_FLUSH |\ diff --git a/sql/sql_base.cc b/sql/sql_base.cc index f68d9a29f05..1564838548f 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -2378,11 +2378,11 @@ open_table_get_mdl_lock(THD *thd, TABLE_LIST *table_list, used in the statement being prepared. */ DBUG_ASSERT(!(flags & (MYSQL_OPEN_TAKE_UPGRADABLE_MDL | - MYSQL_LOCK_IGNORE_FLUSH))); + MYSQL_OPEN_FORCE_SHARED_HIGH_PRIO_MDL))); mdl_request->set_type(MDL_SHARED); } - else if (flags & MYSQL_LOCK_IGNORE_FLUSH) + else if (flags & MYSQL_OPEN_FORCE_SHARED_HIGH_PRIO_MDL) { DBUG_ASSERT(!(flags & MYSQL_OPEN_TAKE_UPGRADABLE_MDL)); mdl_request->set_type(MDL_SHARED_HIGH_PRIO); diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc index 0f84f5e9d73..0f5d51f924d 100644 --- a/sql/sql_delete.cc +++ b/sql/sql_delete.cc @@ -1052,6 +1052,7 @@ static bool mysql_truncate_by_delete(THD *thd, TABLE_LIST *table_list) bool error, save_binlog_row_based= thd->is_current_stmt_binlog_format_row(); DBUG_ENTER("mysql_truncate_by_delete"); table_list->lock_type= TL_WRITE; + table_list->mdl_request.set_type(MDL_SHARED_WRITE); mysql_init_select(thd->lex); thd->clear_current_stmt_binlog_format_row(); /* Delete all rows from table */ diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc index d9c2a84d03e..9f365d0cf2f 100644 --- a/sql/sql_handler.cc +++ b/sql/sql_handler.cc @@ -124,6 +124,7 @@ static void mysql_ha_close_table(THD *thd, TABLE_LIST *tables) { /* Non temporary table. */ tables->table->file->ha_index_or_rnd_end(); + tables->table->open_by_handler= 0; mysql_mutex_lock(&LOCK_open); if (close_thread_table(thd, &tables->table)) { @@ -332,7 +333,8 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen) hash_tables->table->s->tmp_table); /* If it's a temp table, don't reset table->query_id as the table is - being used by this handler. Otherwise, no meaning at all. + being used by this handler. For non-temp tables we use this flag + in asserts. */ hash_tables->table->open_by_handler= 1; diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index fe49962d77a..1906040d5c6 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -7061,6 +7061,9 @@ bool multi_delete_set_locks_and_link_aux_tables(LEX *lex) } walk->updating= target_tbl->updating; walk->lock_type= target_tbl->lock_type; + /* We can assume that tables to be deleted from are locked for write. */ + DBUG_ASSERT(walk->lock_type >= TL_WRITE_ALLOW_WRITE); + walk->mdl_request.set_type(MDL_SHARED_WRITE); target_tbl->correspondent_table= walk; // Remember corresponding table } DBUG_RETURN(FALSE); diff --git a/sql/sql_show.cc b/sql/sql_show.cc index 2238c7bb1ef..2a05cbc561d 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -2928,6 +2928,7 @@ fill_schema_show_cols_or_idxs(THD *thd, TABLE_LIST *tables, lex->sql_command= SQLCOM_SHOW_FIELDS; res= open_normal_and_derived_tables(thd, show_table_list, (MYSQL_LOCK_IGNORE_FLUSH | + MYSQL_OPEN_FORCE_SHARED_HIGH_PRIO_MDL | (can_deadlock ? MYSQL_OPEN_FAIL_ON_MDL_CONFLICT : 0))); lex->sql_command= save_sql_command; @@ -3507,6 +3508,7 @@ int get_all_tables(THD *thd, TABLE_LIST *tables, COND *cond) schema_table->i_s_requested_object; res= open_normal_and_derived_tables(thd, show_table_list, (MYSQL_LOCK_IGNORE_FLUSH | + MYSQL_OPEN_FORCE_SHARED_HIGH_PRIO_MDL | (can_deadlock ? MYSQL_OPEN_FAIL_ON_MDL_CONFLICT : 0))); lex->sql_command= save_sql_command; /* diff --git a/sql/sql_update.cc b/sql/sql_update.cc index 736dceba837..a163dda2c69 100644 --- a/sql/sql_update.cc +++ b/sql/sql_update.cc @@ -1051,6 +1051,11 @@ reopen_tables: If we are using the binary log, we need TL_READ_NO_INSERT to get correct order of statements. Otherwise, we use a TL_READ lock to improve performance. + We don't downgrade metadata lock from SW to SR in this case as + there is no guarantee that the same ticket is not used by + another table instance used by this statement which is going to + be write-locked (for example, trigger to be invoked might try + to update this table). */ tl->lock_type= read_lock_type_for_table(thd, table); tl->updating= 0; diff --git a/sql/sql_view.cc b/sql/sql_view.cc index 931a7adb57f..b9eb6a63552 100644 --- a/sql/sql_view.cc +++ b/sql/sql_view.cc @@ -1347,7 +1347,11 @@ bool mysql_make_view(THD *thd, File_parser *parser, TABLE_LIST *table, anyway. */ for (tbl= view_main_select_tables; tbl; tbl= tbl->next_local) + { tbl->lock_type= table->lock_type; + tbl->mdl_request.set_type((tbl->lock_type >= TL_WRITE_ALLOW_WRITE) ? + MDL_SHARED_WRITE : MDL_SHARED_READ); + } /* If the view is mergeable, we might want to INSERT/UPDATE/DELETE into tables of this view. Preserve the