From 82a96926a77b38a36a031e34b2c4ff3e36406f3d Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Wed, 15 Jun 2016 13:57:44 +0400 Subject: [PATCH] MDEV-9728 - Hard crash in metadata_lock_info metadata_lock_info plugin called MDL_context::find_ticket() to obtain lock duration, which in turn iterates foreign thread private lists. These lists can be updated by owner thread without protection. Fixed by iterating threads (instead of MDL locks and tickets) and obtaining data through APC. Also fixed mdl_iterate_lock() to initialize iterator under prlock protection. --- mysql-test/r/create_or_replace.result | 12 +- mysql-test/t/create_or_replace.test | 10 ++ .../metadata_lock_info/metadata_lock_info.cc | 154 ++++++++++++------ .../r/global_read_lock.result | 2 +- .../t/global_read_lock.test | 2 +- sql/mdl.cc | 4 +- sql/mdl.h | 2 +- 7 files changed, 123 insertions(+), 63 deletions(-) diff --git a/mysql-test/r/create_or_replace.result b/mysql-test/r/create_or_replace.result index ff8170b730982..a8348b8a5d07f 100644 --- a/mysql-test/r/create_or_replace.result +++ b/mysql-test/r/create_or_replace.result @@ -265,10 +265,10 @@ lock table test.t1 write, mysqltest2.t2 write; select * from information_schema.metadata_lock_info; THREAD_ID LOCK_MODE LOCK_DURATION LOCK_TYPE TABLE_SCHEMA TABLE_NAME # MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Global read lock -# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1 # MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Schema metadata lock mysqltest2 # MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Schema metadata lock test # MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock mysqltest2 t2 +# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1 create or replace table test.t1; ERROR 42000: A table must have at least 1 column show tables; @@ -292,10 +292,10 @@ lock table test.t1 write, mysqltest2.t2 write; select * from information_schema.metadata_lock_info; THREAD_ID LOCK_MODE LOCK_DURATION LOCK_TYPE TABLE_SCHEMA TABLE_NAME # MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Global read lock -# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1 # MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Schema metadata lock mysqltest2 # MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Schema metadata lock test # MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock mysqltest2 t2 +# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1 create or replace table test.t1 (a int) select 1 as 'a', 2 as 'a'; ERROR 42S21: Duplicate column name 'a' show tables; @@ -401,29 +401,29 @@ lock table t1 write, t2 read; select * from information_schema.metadata_lock_info; THREAD_ID LOCK_MODE LOCK_DURATION LOCK_TYPE TABLE_SCHEMA TABLE_NAME # MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Global read lock -# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1 # MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Schema metadata lock test +# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1 # MDL_SHARED_READ MDL_EXPLICIT Table metadata lock test t2 create or replace table t1 (i int); select * from information_schema.metadata_lock_info; THREAD_ID LOCK_MODE LOCK_DURATION LOCK_TYPE TABLE_SCHEMA TABLE_NAME # MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Global read lock -# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1 # MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Schema metadata lock test +# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1 # MDL_SHARED_READ MDL_EXPLICIT Table metadata lock test t2 create or replace table t1 like t2; select * from information_schema.metadata_lock_info; THREAD_ID LOCK_MODE LOCK_DURATION LOCK_TYPE TABLE_SCHEMA TABLE_NAME # MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Global read lock -# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1 # MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Schema metadata lock test +# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1 # MDL_SHARED_READ MDL_EXPLICIT Table metadata lock test t2 create or replace table t1 select 1 as f1; select * from information_schema.metadata_lock_info; THREAD_ID LOCK_MODE LOCK_DURATION LOCK_TYPE TABLE_SCHEMA TABLE_NAME # MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Global read lock -# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1 # MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Schema metadata lock test +# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1 # MDL_SHARED_READ MDL_EXPLICIT Table metadata lock test t2 drop table t1; unlock tables; diff --git a/mysql-test/t/create_or_replace.test b/mysql-test/t/create_or_replace.test index 9e37950dbeff3..7bba2b341c043 100644 --- a/mysql-test/t/create_or_replace.test +++ b/mysql-test/t/create_or_replace.test @@ -216,15 +216,18 @@ create table test.t1 (i int); create table mysqltest2.t2 like test.t1; lock table test.t1 write, mysqltest2.t2 write; --replace_column 1 # +--sorted_result select * from information_schema.metadata_lock_info; --error ER_TABLE_MUST_HAVE_COLUMNS create or replace table test.t1; show tables; --replace_column 1 # +--sorted_result select * from information_schema.metadata_lock_info; --error ER_TABLE_MUST_HAVE_COLUMNS create or replace table mysqltest2.t2; --replace_column 1 # +--sorted_result select * from information_schema.metadata_lock_info; create table t1 (i int); drop table t1; @@ -233,15 +236,18 @@ create table test.t1 (i int); create table mysqltest2.t2 like test.t1; lock table test.t1 write, mysqltest2.t2 write; --replace_column 1 # +--sorted_result select * from information_schema.metadata_lock_info; --error ER_DUP_FIELDNAME create or replace table test.t1 (a int) select 1 as 'a', 2 as 'a'; show tables; --replace_column 1 # +--sorted_result select * from information_schema.metadata_lock_info; --error ER_DUP_FIELDNAME create or replace table mysqltest2.t2 (a int) select 1 as 'a', 2 as 'a'; --replace_column 1 # +--sorted_result select * from information_schema.metadata_lock_info; create table t1 (i int); drop table t1; @@ -319,15 +325,19 @@ drop view t1; create table t1 (a int); lock table t1 write, t2 read; --replace_column 1 # +--sorted_result select * from information_schema.metadata_lock_info; create or replace table t1 (i int); --replace_column 1 # +--sorted_result select * from information_schema.metadata_lock_info; create or replace table t1 like t2; --replace_column 1 # +--sorted_result select * from information_schema.metadata_lock_info; create or replace table t1 select 1 as f1; --replace_column 1 # +--sorted_result select * from information_schema.metadata_lock_info; drop table t1; unlock tables; diff --git a/plugin/metadata_lock_info/metadata_lock_info.cc b/plugin/metadata_lock_info/metadata_lock_info.cc index fcfdb59da3056..19c0ee351031b 100644 --- a/plugin/metadata_lock_info/metadata_lock_info.cc +++ b/plugin/metadata_lock_info/metadata_lock_info.cc @@ -67,65 +67,115 @@ static ST_FIELD_INFO i_s_metadata_lock_info_fields_info[] = {NULL, 0, MYSQL_TYPE_STRING, 0, 0, NULL, 0} }; -struct st_i_s_metadata_param + +class Ticket_info: public Apc_target::Apc_call { - THD *thd; +public: + bool timed_out; + THD *request_thd; + THD *target_thd; TABLE *table; + int error; + + Ticket_info(THD *request_thd_arg, TABLE *table_arg): + request_thd(request_thd_arg), table(table_arg), error(0) + { } + + void call_in_target_thread() + { + MDL_ticket *ticket; + int i; + + for (i= 0; i < MDL_DURATION_END; i++) + { + MDL_context::Ticket_iterator it(request_thd->mdl_context.m_tickets[i]); + while ((ticket= it++)) + { + MDL_key *key= ticket->get_key(); + + table->field[0]->store((longlong) target_thd->thread_id, TRUE); + + table->field[1]->set_notnull(); + table->field[1]->store( + metadata_lock_info_lock_mode[(int) ticket->get_type()].str, + metadata_lock_info_lock_mode[(int) ticket->get_type()].length, + system_charset_info); + + table->field[2]->set_notnull(); + table->field[2]->store( + metadata_lock_info_duration[i].str, + metadata_lock_info_duration[i].length, + system_charset_info); + + table->field[3]->set_notnull(); + table->field[3]->store( + metadata_lock_info_lock_name[(int) key->mdl_namespace()].str, + metadata_lock_info_lock_name[(int) key->mdl_namespace()].length, + system_charset_info); + + table->field[4]->set_notnull(); + table->field[4]->store(key->db_name(), key->db_name_length(), + system_charset_info); + + table->field[5]->set_notnull(); + table->field[5]->store(key->name(), key->name_length(), + system_charset_info); + + if ((error= schema_table_store_record(request_thd, table))) + return; + } + } + } }; -int i_s_metadata_lock_info_fill_row( - MDL_ticket *mdl_ticket, - void *arg -) { - st_i_s_metadata_param *param = (st_i_s_metadata_param *) arg; - THD *thd = param->thd; - TABLE *table = param->table; - DBUG_ENTER("i_s_metadata_lock_info_fill_row"); - MDL_request mdl_request; - enum_mdl_duration mdl_duration; - MDL_context *mdl_ctx = mdl_ticket->get_ctx(); - enum_mdl_type mdl_ticket_type = mdl_ticket->get_type(); - MDL_key *mdl_key = mdl_ticket->get_key(); - MDL_key::enum_mdl_namespace mdl_namespace = mdl_key->mdl_namespace(); - mdl_request.init(mdl_key, mdl_ticket_type, MDL_STATEMENT); - mdl_ctx->find_ticket(&mdl_request, &mdl_duration); - table->field[0]->store((longlong) mdl_ctx->get_thread_id(), TRUE); - table->field[1]->set_notnull(); - table->field[1]->store( - metadata_lock_info_lock_mode[(int) mdl_ticket_type].str, - metadata_lock_info_lock_mode[(int) mdl_ticket_type].length, - system_charset_info); - table->field[2]->set_notnull(); - table->field[2]->store( - metadata_lock_info_duration[(int) mdl_duration].str, - metadata_lock_info_duration[(int) mdl_duration].length, - system_charset_info); - table->field[3]->set_notnull(); - table->field[3]->store( - metadata_lock_info_lock_name[(int) mdl_namespace].str, - metadata_lock_info_lock_name[(int) mdl_namespace].length, - system_charset_info); - table->field[4]->set_notnull(); - table->field[4]->store(mdl_key->db_name(), - mdl_key->db_name_length(), system_charset_info); - table->field[5]->set_notnull(); - table->field[5]->store(mdl_key->name(), - mdl_key->name_length(), system_charset_info); - if (schema_table_store_record(thd, table)) - DBUG_RETURN(1); - DBUG_RETURN(0); + +static THD *find_thread(my_thread_id id) +{ + THD *tmp; + + mysql_mutex_lock(&LOCK_thread_count); + I_List_iterator it(threads); + while ((tmp= it++)) + { + if (id == tmp->thread_id) + { + mysql_mutex_lock(&tmp->LOCK_thd_data); + break; + } + } + mysql_mutex_unlock(&LOCK_thread_count); + return tmp; } -int i_s_metadata_lock_info_fill_table( - THD *thd, - TABLE_LIST *tables, - COND *cond -) { - st_i_s_metadata_param param; + +static int i_s_metadata_lock_info_fill_table(THD *thd, TABLE_LIST *tables, + COND *cond) +{ + Ticket_info info(thd, tables->table); + DYNAMIC_ARRAY ids; + THD *tmp; + uint i; DBUG_ENTER("i_s_metadata_lock_info_fill_table"); - param.table = tables->table; - param.thd = thd; - DBUG_RETURN(mdl_iterate(i_s_metadata_lock_info_fill_row, ¶m)); + + /* Gather thread identifiers */ + my_init_dynamic_array(&ids, sizeof(my_thread_id), 512, 1, MYF(0)); + mysql_mutex_lock(&LOCK_thread_count); + I_List_iterator it(threads); + while ((tmp= it++)) + if (tmp != thd && (info.error= insert_dynamic(&ids, &tmp->thread_id))) + break; + mysql_mutex_unlock(&LOCK_thread_count); + + /* Let foreign threads fill info */ + for (i= 0; i < ids.elements && info.error == 0; i++) + if ((info.target_thd= find_thread(*dynamic_element(&ids, i, my_thread_id*)))) + info.target_thd->apc_target.make_apc_call(thd, &info, INT_MAX, + &info.timed_out); + + delete_dynamic(&ids); + if (info.error == 0) + info.call_in_target_thread(); + DBUG_RETURN(info.error); } static int i_s_metadata_lock_info_init( diff --git a/plugin/metadata_lock_info/mysql-test/metadata_lock_info/r/global_read_lock.result b/plugin/metadata_lock_info/mysql-test/metadata_lock_info/r/global_read_lock.result index 9840aeecf973c..61b0a913ca497 100644 --- a/plugin/metadata_lock_info/mysql-test/metadata_lock_info/r/global_read_lock.result +++ b/plugin/metadata_lock_info/mysql-test/metadata_lock_info/r/global_read_lock.result @@ -1,7 +1,7 @@ SELECT lock_mode, lock_duration, lock_type, table_schema, table_name FROM information_schema.metadata_lock_info; lock_mode lock_duration lock_type table_schema table_name FLUSH TABLES WITH READ LOCK; -SELECT lock_mode, lock_duration, lock_type, table_schema, table_name FROM information_schema.metadata_lock_info; +SELECT lock_mode, lock_duration, lock_type, table_schema, table_name FROM information_schema.metadata_lock_info ORDER BY lock_type DESC; lock_mode lock_duration lock_type table_schema table_name MDL_SHARED MDL_EXPLICIT Global read lock MDL_SHARED MDL_EXPLICIT Commit lock diff --git a/plugin/metadata_lock_info/mysql-test/metadata_lock_info/t/global_read_lock.test b/plugin/metadata_lock_info/mysql-test/metadata_lock_info/t/global_read_lock.test index f221191255e40..74fa5e3e1fd3a 100644 --- a/plugin/metadata_lock_info/mysql-test/metadata_lock_info/t/global_read_lock.test +++ b/plugin/metadata_lock_info/mysql-test/metadata_lock_info/t/global_read_lock.test @@ -1,5 +1,5 @@ SELECT lock_mode, lock_duration, lock_type, table_schema, table_name FROM information_schema.metadata_lock_info; FLUSH TABLES WITH READ LOCK; -SELECT lock_mode, lock_duration, lock_type, table_schema, table_name FROM information_schema.metadata_lock_info; +SELECT lock_mode, lock_duration, lock_type, table_schema, table_name FROM information_schema.metadata_lock_info ORDER BY lock_type DESC; UNLOCK TABLES; SELECT lock_mode, lock_duration, lock_type, table_schema, table_name FROM information_schema.metadata_lock_info; diff --git a/sql/mdl.cc b/sql/mdl.cc index b94a3710fd110..28d2006b023a7 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -706,10 +706,10 @@ static inline int mdl_iterate_lock(MDL_lock *lock, int (*callback)(MDL_ticket *ticket, void *arg), void *arg) { - MDL_lock::Ticket_iterator ticket_it(lock->m_granted); - MDL_ticket *ticket; int res= 0; mysql_prlock_rdlock(&lock->m_rwlock); + MDL_lock::Ticket_iterator ticket_it(lock->m_granted); + MDL_ticket *ticket; while ((ticket= ticket_it++) && !(res= callback(ticket, arg))) /* no-op */; mysql_prlock_unlock(&lock->m_rwlock); return res; diff --git a/sql/mdl.h b/sql/mdl.h index c4d792acd290e..ec7b633a67d9e 100644 --- a/sql/mdl.h +++ b/sql/mdl.h @@ -953,7 +953,7 @@ class MDL_context MDL_context &operator=(MDL_context &rhs); /* not implemented */ /* metadata_lock_info plugin */ - friend int i_s_metadata_lock_info_fill_row(MDL_ticket*, void*); + friend class Ticket_info; };