diff --git a/mysql-test/r/mdl_sync.result b/mysql-test/r/mdl_sync.result index e5447c32b7d..130a8f22710 100644 --- a/mysql-test/r/mdl_sync.result +++ b/mysql-test/r/mdl_sync.result @@ -254,3 +254,40 @@ commit; # Switching to connection 'default'. # Clean-up drop table t1; +# +# Bug#48210 FLUSH TABLES WITH READ LOCK deadlocks +# against concurrent CREATE PROCEDURE +# +# Test 1: CREATE PROCEDURE +# Connection 1 +# Start CREATE PROCEDURE and open mysql.proc +SET DEBUG_SYNC= 'after_open_table_mdl_shared SIGNAL table_opened WAIT_FOR grlwait'; +CREATE PROCEDURE p1() SELECT 1; +# Connection 2 +SET DEBUG_SYNC= 'now WAIT_FOR table_opened'; +# Check that FLUSH must wait to get the GRL +# and let CREATE PROCEDURE continue +SET DEBUG_SYNC= 'wait_lock_global_read_lock SIGNAL grlwait'; +FLUSH TABLES WITH READ LOCK; +# Connection 1 +# Connection 2 +UNLOCK TABLES; +# Connection 1 +DROP PROCEDURE p1; +SET DEBUG_SYNC= 'RESET'; +# Test 2: CREATE USER +# Start CREATE USER and open the grant tables +SET DEBUG_SYNC= 'after_open_table_mdl_shared SIGNAL table_opened WAIT_FOR grlwait'; +CREATE USER 'user_1@localhost'; +# Connection 2 +SET DEBUG_SYNC= 'now WAIT_FOR table_opened'; +# Check that FLUSH must wait to get the GRL +# and let CREATE USER continue +SET DEBUG_SYNC= 'wait_lock_global_read_lock SIGNAL grlwait'; +FLUSH TABLES WITH READ LOCK; +# Connection 1 +# Connection 2 +UNLOCK TABLES; +# Connection 1 +DROP USER 'user_1@localhost'; +SET DEBUG_SYNC= 'RESET'; diff --git a/mysql-test/t/mdl_sync.test b/mysql-test/t/mdl_sync.test index fd66f6d539d..9422c67cb6f 100644 --- a/mysql-test/t/mdl_sync.test +++ b/mysql-test/t/mdl_sync.test @@ -475,6 +475,75 @@ disconnect con46673; drop table t1; +--echo # +--echo # Bug#48210 FLUSH TABLES WITH READ LOCK deadlocks +--echo # against concurrent CREATE PROCEDURE +--echo # + +connect (con2, localhost, root); + +--echo # Test 1: CREATE PROCEDURE + +--echo # Connection 1 +connection default; +--echo # Start CREATE PROCEDURE and open mysql.proc +SET DEBUG_SYNC= 'after_open_table_mdl_shared SIGNAL table_opened WAIT_FOR grlwait'; +--send CREATE PROCEDURE p1() SELECT 1 + +--echo # Connection 2 +connection con2; +SET DEBUG_SYNC= 'now WAIT_FOR table_opened'; +--echo # Check that FLUSH must wait to get the GRL +--echo # and let CREATE PROCEDURE continue +SET DEBUG_SYNC= 'wait_lock_global_read_lock SIGNAL grlwait'; +--send FLUSH TABLES WITH READ LOCK + +--echo # Connection 1 +connection default; +--reap + +--echo # Connection 2 +connection con2; +--reap +UNLOCK TABLES; + +--echo # Connection 1 +connection default; +DROP PROCEDURE p1; +SET DEBUG_SYNC= 'RESET'; + +--echo # Test 2: CREATE USER + +connection default; +--echo # Start CREATE USER and open the grant tables +SET DEBUG_SYNC= 'after_open_table_mdl_shared SIGNAL table_opened WAIT_FOR grlwait'; +--send CREATE USER 'user_1@localhost' + +--echo # Connection 2 +connection con2; +SET DEBUG_SYNC= 'now WAIT_FOR table_opened'; +--echo # Check that FLUSH must wait to get the GRL +--echo # and let CREATE USER continue +SET DEBUG_SYNC= 'wait_lock_global_read_lock SIGNAL grlwait'; +--send FLUSH TABLES WITH READ LOCK + +--echo # Connection 1 +connection default; +--reap + +--echo # Connection 2 +connection con2; +--reap +UNLOCK TABLES; + +--echo # Connection 1 +connection default; +DROP USER 'user_1@localhost'; +SET DEBUG_SYNC= 'RESET'; + +disconnect con2; + + # 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 b64f7b0b44c..98217254240 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -74,6 +74,7 @@ */ #include "mysql_priv.h" +#include "debug_sync.h" #include #include @@ -1119,9 +1120,33 @@ bool lock_global_read_lock(THD *thd) if (!thd->global_read_lock) { const char *old_message; + const char *new_message= "Waiting to get readlock"; (void) pthread_mutex_lock(&LOCK_global_read_lock); + +#if defined(ENABLED_DEBUG_SYNC) + /* + The below sync point fires if we have to wait for + protect_against_global_read_lock. + + WARNING: Beware to use WAIT_FOR with this sync point. We hold + LOCK_global_read_lock here. + + Call the sync point before calling enter_cond() as it does use + enter_cond() and exit_cond() itself if a WAIT_FOR action is + executed in spite of the above warning. + + Pre-set proc_info so that it is available immediately after the + sync point sends a SIGNAL. This makes tests more reliable. + */ + if (protect_against_global_read_lock) + { + thd_proc_info(thd, new_message); + DEBUG_SYNC(thd, "wait_lock_global_read_lock"); + } +#endif /* defined(ENABLED_DEBUG_SYNC) */ + old_message=thd->enter_cond(&COND_global_read_lock, &LOCK_global_read_lock, - "Waiting to get readlock"); + new_message); DBUG_PRINT("info", ("waiting_for: %d protect_against: %d", waiting_for_read_lock, protect_against_global_read_lock)); diff --git a/sql/sql_class.h b/sql/sql_class.h index 889d7c5472b..4bfd3e66438 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -3341,6 +3341,16 @@ public: */ #define CF_DIAGNOSTIC_STMT (1U << 8) +/** + SQL statements that must be protected against impending global read lock + to prevent deadlock. This deadlock could otherwise happen if the statement + starts waiting for the GRL to go away inside mysql_lock_tables while at the + same time having "old" opened tables. The thread holding the GRL can be + waiting for these "old" opened tables to be closed, causing a deadlock + (FLUSH TABLES WITH READ LOCK). + */ +#define CF_PROTECT_AGAINST_GRL (1U << 10) + /* Bits in server_command_flags */ /** diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index e462556c133..289b6ce0da8 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -182,14 +182,15 @@ void init_update_queries(void) memset(sql_command_flags, 0, sizeof(sql_command_flags)); sql_command_flags[SQLCOM_CREATE_TABLE]= CF_CHANGES_DATA | CF_REEXECUTION_FRAGILE | - CF_AUTO_COMMIT_TRANS; + CF_AUTO_COMMIT_TRANS | CF_PROTECT_AGAINST_GRL; sql_command_flags[SQLCOM_CREATE_INDEX]= CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_ALTER_TABLE]= CF_CHANGES_DATA | CF_WRITE_LOGS_COMMAND | - CF_AUTO_COMMIT_TRANS; + CF_AUTO_COMMIT_TRANS | CF_PROTECT_AGAINST_GRL; sql_command_flags[SQLCOM_TRUNCATE]= CF_CHANGES_DATA | CF_WRITE_LOGS_COMMAND | CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_DROP_TABLE]= CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS; - sql_command_flags[SQLCOM_LOAD]= CF_CHANGES_DATA | CF_REEXECUTION_FRAGILE; + sql_command_flags[SQLCOM_LOAD]= CF_CHANGES_DATA | CF_REEXECUTION_FRAGILE | + CF_PROTECT_AGAINST_GRL; sql_command_flags[SQLCOM_CREATE_DB]= CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_DROP_DB]= CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_RENAME_TABLE]= CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS; @@ -207,17 +208,17 @@ void init_update_queries(void) sql_command_flags[SQLCOM_DROP_TRIGGER]= CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_UPDATE]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT | - CF_REEXECUTION_FRAGILE; + CF_REEXECUTION_FRAGILE | CF_PROTECT_AGAINST_GRL; sql_command_flags[SQLCOM_UPDATE_MULTI]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT | - CF_REEXECUTION_FRAGILE; + CF_REEXECUTION_FRAGILE | CF_PROTECT_AGAINST_GRL; sql_command_flags[SQLCOM_INSERT]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT | - CF_REEXECUTION_FRAGILE; + CF_REEXECUTION_FRAGILE | CF_PROTECT_AGAINST_GRL; sql_command_flags[SQLCOM_INSERT_SELECT]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT | - CF_REEXECUTION_FRAGILE; + CF_REEXECUTION_FRAGILE | CF_PROTECT_AGAINST_GRL; sql_command_flags[SQLCOM_DELETE]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT | - CF_REEXECUTION_FRAGILE; + CF_REEXECUTION_FRAGILE | CF_PROTECT_AGAINST_GRL; sql_command_flags[SQLCOM_DELETE_MULTI]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT | - CF_REEXECUTION_FRAGILE; + CF_REEXECUTION_FRAGILE | CF_PROTECT_AGAINST_GRL; sql_command_flags[SQLCOM_REPLACE]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT | CF_REEXECUTION_FRAGILE; sql_command_flags[SQLCOM_REPLACE_SELECT]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT | @@ -276,20 +277,21 @@ void init_update_queries(void) CF_REEXECUTION_FRAGILE); - sql_command_flags[SQLCOM_CREATE_USER]= CF_CHANGES_DATA; - sql_command_flags[SQLCOM_RENAME_USER]= CF_CHANGES_DATA; - sql_command_flags[SQLCOM_DROP_USER]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_CREATE_USER]= CF_CHANGES_DATA | CF_PROTECT_AGAINST_GRL; + sql_command_flags[SQLCOM_RENAME_USER]= CF_CHANGES_DATA | CF_PROTECT_AGAINST_GRL; + sql_command_flags[SQLCOM_DROP_USER]= CF_CHANGES_DATA | CF_PROTECT_AGAINST_GRL; sql_command_flags[SQLCOM_GRANT]= CF_CHANGES_DATA; sql_command_flags[SQLCOM_REVOKE]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_REVOKE_ALL]= CF_PROTECT_AGAINST_GRL; sql_command_flags[SQLCOM_ALTER_DB]= CF_CHANGES_DATA; sql_command_flags[SQLCOM_CREATE_FUNCTION]= CF_CHANGES_DATA; - sql_command_flags[SQLCOM_DROP_FUNCTION]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_DROP_FUNCTION]= CF_CHANGES_DATA | CF_PROTECT_AGAINST_GRL; sql_command_flags[SQLCOM_OPTIMIZE]= CF_CHANGES_DATA; - sql_command_flags[SQLCOM_CREATE_PROCEDURE]= CF_CHANGES_DATA; - sql_command_flags[SQLCOM_CREATE_SPFUNCTION]= CF_CHANGES_DATA; - sql_command_flags[SQLCOM_DROP_PROCEDURE]= CF_CHANGES_DATA; - sql_command_flags[SQLCOM_ALTER_PROCEDURE]= CF_CHANGES_DATA; - sql_command_flags[SQLCOM_ALTER_FUNCTION]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_CREATE_PROCEDURE]= CF_CHANGES_DATA | CF_PROTECT_AGAINST_GRL; + sql_command_flags[SQLCOM_CREATE_SPFUNCTION]= CF_CHANGES_DATA | CF_PROTECT_AGAINST_GRL; + sql_command_flags[SQLCOM_DROP_PROCEDURE]= CF_CHANGES_DATA | CF_PROTECT_AGAINST_GRL; + sql_command_flags[SQLCOM_ALTER_PROCEDURE]= CF_CHANGES_DATA | CF_PROTECT_AGAINST_GRL; + sql_command_flags[SQLCOM_ALTER_FUNCTION]= CF_CHANGES_DATA | CF_PROTECT_AGAINST_GRL; sql_command_flags[SQLCOM_INSTALL_PLUGIN]= CF_CHANGES_DATA; sql_command_flags[SQLCOM_UNINSTALL_PLUGIN]= CF_CHANGES_DATA; @@ -1969,6 +1971,17 @@ mysql_execute_command(THD *thd) thd->mdl_context.release_all_locks(); } + /* + Check if this command needs protection against the global read lock + to avoid deadlock. See CF_PROTECT_AGAINST_GRL. + start_waiting_global_read_lock() is called at the end of + mysql_execute_command(). + */ + if (((sql_command_flags[lex->sql_command] & CF_PROTECT_AGAINST_GRL) != 0) && + !thd->locked_tables_mode) + if (wait_if_global_read_lock(thd, FALSE, TRUE)) + goto error; + switch (lex->sql_command) { case SQLCOM_SHOW_EVENTS: @@ -2309,12 +2322,9 @@ case SQLCOM_PREPARE: start_waiting_global_read_lock(). We protect the normal CREATE TABLE in the same way. That way we avoid that a new table is created during a global read lock. + Protection against grl is covered by the CF_PROTECT_AGAINST_GRL flag. */ - if (!thd->locked_tables_mode && wait_if_global_read_lock(thd, 0, 1)) - { - res= 1; - goto end_with_restore_list; - } + #ifdef WITH_PARTITION_STORAGE_ENGINE { partition_info *part_info= thd->lex->part_info; @@ -2617,12 +2627,6 @@ end_with_restore_list: "INDEX DIRECTORY"); create_info.data_file_name= create_info.index_file_name= NULL; - if (!thd->locked_tables_mode && wait_if_global_read_lock(thd, 0, 1)) - { - res= 1; - break; - } - thd->enable_slow_log= opt_log_slow_admin_statements; res= mysql_alter_table(thd, select_lex->db, lex->name.str, &create_info, @@ -2852,8 +2856,6 @@ end_with_restore_list: DBUG_ASSERT(first_table == all_tables && first_table != 0); if (update_precheck(thd, all_tables)) break; - if (!thd->locked_tables_mode && wait_if_global_read_lock(thd, 0, 1)) - goto error; DBUG_ASSERT(select_lex->offset_limit == 0); unit->set_limit(select_lex); MYSQL_UPDATE_START(thd->query()); @@ -2884,15 +2886,6 @@ end_with_restore_list: else res= 0; - /* - Protection might have already been risen if its a fall through - from the SQLCOM_UPDATE case above. - */ - if (!thd->locked_tables_mode && - lex->sql_command == SQLCOM_UPDATE_MULTI && - wait_if_global_read_lock(thd, 0, 1)) - goto error; - res= mysql_multi_update_prepare(thd); #ifdef HAVE_REPLICATION @@ -2992,11 +2985,6 @@ end_with_restore_list: if ((res= insert_precheck(thd, all_tables))) break; - if (!thd->locked_tables_mode && wait_if_global_read_lock(thd, 0, 1)) - { - res= 1; - break; - } MYSQL_INSERT_START(thd->query()); res= mysql_insert(thd, all_tables, lex->field_list, lex->many_values, lex->update_list, lex->value_list, @@ -3031,11 +3019,6 @@ end_with_restore_list: unit->set_limit(select_lex); - if (!thd->locked_tables_mode && wait_if_global_read_lock(thd, 0, 1)) - { - res= 1; - break; - } if (!(res= open_and_lock_tables(thd, all_tables))) { MYSQL_INSERT_SELECT_START(thd->query()); @@ -3113,11 +3096,6 @@ end_with_restore_list: DBUG_ASSERT(select_lex->offset_limit == 0); unit->set_limit(select_lex); - if (!thd->locked_tables_mode && wait_if_global_read_lock(thd, 0, 1)) - { - res= 1; - break; - } MYSQL_DELETE_START(thd->query()); res = mysql_delete(thd, all_tables, select_lex->where, &select_lex->order_list, @@ -3133,12 +3111,6 @@ end_with_restore_list: (TABLE_LIST *)thd->lex->auxiliary_table_list.first; multi_delete *del_result; - if (!thd->locked_tables_mode && wait_if_global_read_lock(thd, 0, 1)) - { - res= 1; - break; - } - if ((res= multi_delete_precheck(thd, all_tables))) break; @@ -3277,9 +3249,6 @@ end_with_restore_list: if (check_one_table_access(thd, privilege, all_tables)) goto error; - if (!thd->locked_tables_mode && wait_if_global_read_lock(thd, 0, 1)) - goto error; - res= mysql_load(thd, lex->exchange, first_table, lex->field_list, lex->update_list, lex->value_list, lex->duplicates, lex->ignore, (bool) lex->local_file);