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

@@ -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;

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;

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<THD> 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, &param));
+
+  /* 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<THD> 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(

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		

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;

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;

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;
 };