Skip to content

Commit 82a9692

Browse files
committed
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.
1 parent 1531321 commit 82a9692

File tree

7 files changed

+123
-63
lines changed

7 files changed

+123
-63
lines changed

mysql-test/r/create_or_replace.result

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -265,10 +265,10 @@ lock table test.t1 write, mysqltest2.t2 write;
265265
select * from information_schema.metadata_lock_info;
266266
THREAD_ID LOCK_MODE LOCK_DURATION LOCK_TYPE TABLE_SCHEMA TABLE_NAME
267267
# MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Global read lock
268-
# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1
269268
# MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Schema metadata lock mysqltest2
270269
# MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Schema metadata lock test
271270
# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock mysqltest2 t2
271+
# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1
272272
create or replace table test.t1;
273273
ERROR 42000: A table must have at least 1 column
274274
show tables;
@@ -292,10 +292,10 @@ lock table test.t1 write, mysqltest2.t2 write;
292292
select * from information_schema.metadata_lock_info;
293293
THREAD_ID LOCK_MODE LOCK_DURATION LOCK_TYPE TABLE_SCHEMA TABLE_NAME
294294
# MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Global read lock
295-
# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1
296295
# MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Schema metadata lock mysqltest2
297296
# MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Schema metadata lock test
298297
# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock mysqltest2 t2
298+
# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1
299299
create or replace table test.t1 (a int) select 1 as 'a', 2 as 'a';
300300
ERROR 42S21: Duplicate column name 'a'
301301
show tables;
@@ -401,29 +401,29 @@ lock table t1 write, t2 read;
401401
select * from information_schema.metadata_lock_info;
402402
THREAD_ID LOCK_MODE LOCK_DURATION LOCK_TYPE TABLE_SCHEMA TABLE_NAME
403403
# MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Global read lock
404-
# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1
405404
# MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Schema metadata lock test
405+
# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1
406406
# MDL_SHARED_READ MDL_EXPLICIT Table metadata lock test t2
407407
create or replace table t1 (i int);
408408
select * from information_schema.metadata_lock_info;
409409
THREAD_ID LOCK_MODE LOCK_DURATION LOCK_TYPE TABLE_SCHEMA TABLE_NAME
410410
# MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Global read lock
411-
# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1
412411
# MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Schema metadata lock test
412+
# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1
413413
# MDL_SHARED_READ MDL_EXPLICIT Table metadata lock test t2
414414
create or replace table t1 like t2;
415415
select * from information_schema.metadata_lock_info;
416416
THREAD_ID LOCK_MODE LOCK_DURATION LOCK_TYPE TABLE_SCHEMA TABLE_NAME
417417
# MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Global read lock
418-
# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1
419418
# MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Schema metadata lock test
419+
# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1
420420
# MDL_SHARED_READ MDL_EXPLICIT Table metadata lock test t2
421421
create or replace table t1 select 1 as f1;
422422
select * from information_schema.metadata_lock_info;
423423
THREAD_ID LOCK_MODE LOCK_DURATION LOCK_TYPE TABLE_SCHEMA TABLE_NAME
424424
# MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Global read lock
425-
# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1
426425
# MDL_INTENTION_EXCLUSIVE MDL_EXPLICIT Schema metadata lock test
426+
# MDL_SHARED_NO_READ_WRITE MDL_EXPLICIT Table metadata lock test t1
427427
# MDL_SHARED_READ MDL_EXPLICIT Table metadata lock test t2
428428
drop table t1;
429429
unlock tables;

mysql-test/t/create_or_replace.test

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,15 +216,18 @@ create table test.t1 (i int);
216216
create table mysqltest2.t2 like test.t1;
217217
lock table test.t1 write, mysqltest2.t2 write;
218218
--replace_column 1 #
219+
--sorted_result
219220
select * from information_schema.metadata_lock_info;
220221
--error ER_TABLE_MUST_HAVE_COLUMNS
221222
create or replace table test.t1;
222223
show tables;
223224
--replace_column 1 #
225+
--sorted_result
224226
select * from information_schema.metadata_lock_info;
225227
--error ER_TABLE_MUST_HAVE_COLUMNS
226228
create or replace table mysqltest2.t2;
227229
--replace_column 1 #
230+
--sorted_result
228231
select * from information_schema.metadata_lock_info;
229232
create table t1 (i int);
230233
drop table t1;
@@ -233,15 +236,18 @@ create table test.t1 (i int);
233236
create table mysqltest2.t2 like test.t1;
234237
lock table test.t1 write, mysqltest2.t2 write;
235238
--replace_column 1 #
239+
--sorted_result
236240
select * from information_schema.metadata_lock_info;
237241
--error ER_DUP_FIELDNAME
238242
create or replace table test.t1 (a int) select 1 as 'a', 2 as 'a';
239243
show tables;
240244
--replace_column 1 #
245+
--sorted_result
241246
select * from information_schema.metadata_lock_info;
242247
--error ER_DUP_FIELDNAME
243248
create or replace table mysqltest2.t2 (a int) select 1 as 'a', 2 as 'a';
244249
--replace_column 1 #
250+
--sorted_result
245251
select * from information_schema.metadata_lock_info;
246252
create table t1 (i int);
247253
drop table t1;
@@ -319,15 +325,19 @@ drop view t1;
319325
create table t1 (a int);
320326
lock table t1 write, t2 read;
321327
--replace_column 1 #
328+
--sorted_result
322329
select * from information_schema.metadata_lock_info;
323330
create or replace table t1 (i int);
324331
--replace_column 1 #
332+
--sorted_result
325333
select * from information_schema.metadata_lock_info;
326334
create or replace table t1 like t2;
327335
--replace_column 1 #
336+
--sorted_result
328337
select * from information_schema.metadata_lock_info;
329338
create or replace table t1 select 1 as f1;
330339
--replace_column 1 #
340+
--sorted_result
331341
select * from information_schema.metadata_lock_info;
332342
drop table t1;
333343
unlock tables;

plugin/metadata_lock_info/metadata_lock_info.cc

Lines changed: 102 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -67,65 +67,115 @@ static ST_FIELD_INFO i_s_metadata_lock_info_fields_info[] =
6767
{NULL, 0, MYSQL_TYPE_STRING, 0, 0, NULL, 0}
6868
};
6969

70-
struct st_i_s_metadata_param
70+
71+
class Ticket_info: public Apc_target::Apc_call
7172
{
72-
THD *thd;
73+
public:
74+
bool timed_out;
75+
THD *request_thd;
76+
THD *target_thd;
7377
TABLE *table;
78+
int error;
79+
80+
Ticket_info(THD *request_thd_arg, TABLE *table_arg):
81+
request_thd(request_thd_arg), table(table_arg), error(0)
82+
{ }
83+
84+
void call_in_target_thread()
85+
{
86+
MDL_ticket *ticket;
87+
int i;
88+
89+
for (i= 0; i < MDL_DURATION_END; i++)
90+
{
91+
MDL_context::Ticket_iterator it(request_thd->mdl_context.m_tickets[i]);
92+
while ((ticket= it++))
93+
{
94+
MDL_key *key= ticket->get_key();
95+
96+
table->field[0]->store((longlong) target_thd->thread_id, TRUE);
97+
98+
table->field[1]->set_notnull();
99+
table->field[1]->store(
100+
metadata_lock_info_lock_mode[(int) ticket->get_type()].str,
101+
metadata_lock_info_lock_mode[(int) ticket->get_type()].length,
102+
system_charset_info);
103+
104+
table->field[2]->set_notnull();
105+
table->field[2]->store(
106+
metadata_lock_info_duration[i].str,
107+
metadata_lock_info_duration[i].length,
108+
system_charset_info);
109+
110+
table->field[3]->set_notnull();
111+
table->field[3]->store(
112+
metadata_lock_info_lock_name[(int) key->mdl_namespace()].str,
113+
metadata_lock_info_lock_name[(int) key->mdl_namespace()].length,
114+
system_charset_info);
115+
116+
table->field[4]->set_notnull();
117+
table->field[4]->store(key->db_name(), key->db_name_length(),
118+
system_charset_info);
119+
120+
table->field[5]->set_notnull();
121+
table->field[5]->store(key->name(), key->name_length(),
122+
system_charset_info);
123+
124+
if ((error= schema_table_store_record(request_thd, table)))
125+
return;
126+
}
127+
}
128+
}
74129
};
75130

76-
int i_s_metadata_lock_info_fill_row(
77-
MDL_ticket *mdl_ticket,
78-
void *arg
79-
) {
80-
st_i_s_metadata_param *param = (st_i_s_metadata_param *) arg;
81-
THD *thd = param->thd;
82-
TABLE *table = param->table;
83-
DBUG_ENTER("i_s_metadata_lock_info_fill_row");
84-
MDL_request mdl_request;
85-
enum_mdl_duration mdl_duration;
86-
MDL_context *mdl_ctx = mdl_ticket->get_ctx();
87-
enum_mdl_type mdl_ticket_type = mdl_ticket->get_type();
88-
MDL_key *mdl_key = mdl_ticket->get_key();
89-
MDL_key::enum_mdl_namespace mdl_namespace = mdl_key->mdl_namespace();
90-
mdl_request.init(mdl_key, mdl_ticket_type, MDL_STATEMENT);
91-
mdl_ctx->find_ticket(&mdl_request, &mdl_duration);
92-
table->field[0]->store((longlong) mdl_ctx->get_thread_id(), TRUE);
93-
table->field[1]->set_notnull();
94-
table->field[1]->store(
95-
metadata_lock_info_lock_mode[(int) mdl_ticket_type].str,
96-
metadata_lock_info_lock_mode[(int) mdl_ticket_type].length,
97-
system_charset_info);
98-
table->field[2]->set_notnull();
99-
table->field[2]->store(
100-
metadata_lock_info_duration[(int) mdl_duration].str,
101-
metadata_lock_info_duration[(int) mdl_duration].length,
102-
system_charset_info);
103-
table->field[3]->set_notnull();
104-
table->field[3]->store(
105-
metadata_lock_info_lock_name[(int) mdl_namespace].str,
106-
metadata_lock_info_lock_name[(int) mdl_namespace].length,
107-
system_charset_info);
108-
table->field[4]->set_notnull();
109-
table->field[4]->store(mdl_key->db_name(),
110-
mdl_key->db_name_length(), system_charset_info);
111-
table->field[5]->set_notnull();
112-
table->field[5]->store(mdl_key->name(),
113-
mdl_key->name_length(), system_charset_info);
114-
if (schema_table_store_record(thd, table))
115-
DBUG_RETURN(1);
116-
DBUG_RETURN(0);
131+
132+
static THD *find_thread(my_thread_id id)
133+
{
134+
THD *tmp;
135+
136+
mysql_mutex_lock(&LOCK_thread_count);
137+
I_List_iterator<THD> it(threads);
138+
while ((tmp= it++))
139+
{
140+
if (id == tmp->thread_id)
141+
{
142+
mysql_mutex_lock(&tmp->LOCK_thd_data);
143+
break;
144+
}
145+
}
146+
mysql_mutex_unlock(&LOCK_thread_count);
147+
return tmp;
117148
}
118149

119-
int i_s_metadata_lock_info_fill_table(
120-
THD *thd,
121-
TABLE_LIST *tables,
122-
COND *cond
123-
) {
124-
st_i_s_metadata_param param;
150+
151+
static int i_s_metadata_lock_info_fill_table(THD *thd, TABLE_LIST *tables,
152+
COND *cond)
153+
{
154+
Ticket_info info(thd, tables->table);
155+
DYNAMIC_ARRAY ids;
156+
THD *tmp;
157+
uint i;
125158
DBUG_ENTER("i_s_metadata_lock_info_fill_table");
126-
param.table = tables->table;
127-
param.thd = thd;
128-
DBUG_RETURN(mdl_iterate(i_s_metadata_lock_info_fill_row, &param));
159+
160+
/* Gather thread identifiers */
161+
my_init_dynamic_array(&ids, sizeof(my_thread_id), 512, 1, MYF(0));
162+
mysql_mutex_lock(&LOCK_thread_count);
163+
I_List_iterator<THD> it(threads);
164+
while ((tmp= it++))
165+
if (tmp != thd && (info.error= insert_dynamic(&ids, &tmp->thread_id)))
166+
break;
167+
mysql_mutex_unlock(&LOCK_thread_count);
168+
169+
/* Let foreign threads fill info */
170+
for (i= 0; i < ids.elements && info.error == 0; i++)
171+
if ((info.target_thd= find_thread(*dynamic_element(&ids, i, my_thread_id*))))
172+
info.target_thd->apc_target.make_apc_call(thd, &info, INT_MAX,
173+
&info.timed_out);
174+
175+
delete_dynamic(&ids);
176+
if (info.error == 0)
177+
info.call_in_target_thread();
178+
DBUG_RETURN(info.error);
129179
}
130180

131181
static int i_s_metadata_lock_info_init(

plugin/metadata_lock_info/mysql-test/metadata_lock_info/r/global_read_lock.result

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
SELECT lock_mode, lock_duration, lock_type, table_schema, table_name FROM information_schema.metadata_lock_info;
22
lock_mode lock_duration lock_type table_schema table_name
33
FLUSH TABLES WITH READ LOCK;
4-
SELECT lock_mode, lock_duration, lock_type, table_schema, table_name FROM information_schema.metadata_lock_info;
4+
SELECT lock_mode, lock_duration, lock_type, table_schema, table_name FROM information_schema.metadata_lock_info ORDER BY lock_type DESC;
55
lock_mode lock_duration lock_type table_schema table_name
66
MDL_SHARED MDL_EXPLICIT Global read lock
77
MDL_SHARED MDL_EXPLICIT Commit lock
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
SELECT lock_mode, lock_duration, lock_type, table_schema, table_name FROM information_schema.metadata_lock_info;
22
FLUSH TABLES WITH READ LOCK;
3-
SELECT lock_mode, lock_duration, lock_type, table_schema, table_name FROM information_schema.metadata_lock_info;
3+
SELECT lock_mode, lock_duration, lock_type, table_schema, table_name FROM information_schema.metadata_lock_info ORDER BY lock_type DESC;
44
UNLOCK TABLES;
55
SELECT lock_mode, lock_duration, lock_type, table_schema, table_name FROM information_schema.metadata_lock_info;

sql/mdl.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -706,10 +706,10 @@ static inline int mdl_iterate_lock(MDL_lock *lock,
706706
int (*callback)(MDL_ticket *ticket, void *arg),
707707
void *arg)
708708
{
709-
MDL_lock::Ticket_iterator ticket_it(lock->m_granted);
710-
MDL_ticket *ticket;
711709
int res= 0;
712710
mysql_prlock_rdlock(&lock->m_rwlock);
711+
MDL_lock::Ticket_iterator ticket_it(lock->m_granted);
712+
MDL_ticket *ticket;
713713
while ((ticket= ticket_it++) && !(res= callback(ticket, arg))) /* no-op */;
714714
mysql_prlock_unlock(&lock->m_rwlock);
715715
return res;

sql/mdl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -953,7 +953,7 @@ class MDL_context
953953
MDL_context &operator=(MDL_context &rhs); /* not implemented */
954954

955955
/* metadata_lock_info plugin */
956-
friend int i_s_metadata_lock_info_fill_row(MDL_ticket*, void*);
956+
friend class Ticket_info;
957957
};
958958

959959

0 commit comments

Comments
 (0)