1berry/MariaDB-server
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Bug#23713 LOCK TABLES + CREATE TRIGGER + FLUSH TABLES WITH READ LOCK = deadlock

Browse Source 
This bug is actually two bugs in one, one of which is CREATE TRIGGER under
LOCK TABLES and the other is CREATE TRIGGER under LOCK TABLES simultaneous
to a FLUSH TABLES WITH READ LOCK (global read lock). Both situations could
lead to a server crash or deadlock.

The first problem arises from the fact that when under LOCK TABLES, if the
table is in the set of locked tables, the table is already open and it doesn't
need to be reopened (not a placeholder). Also in this case, if the table is
not write locked, a exclusive lock can't be acquired because of a possible
deadlock with another thread also holding a (read) lock on the table. The
second issue arises from the fact that one should never wait for a global
read lock if it's holding any locked tables, because the global read lock
is waiting for these tables and this leads to a circular wait deadlock.

The solution for the first case is to check if the table is write locked
and upgraded the write lock to a exclusive lock and fail otherwise for non
write locked tables. Grabbin the exclusive lock in this case also means
to ensure that the table is opened only by the calling thread. The second
issue is partly fixed by not waiting for the global read lock if the thread
is holding any locked tables.

The second issue is only partly addressed in this patch because it turned
out to be much wider and also affects other DDL statements. Reported as
Bug#32395
This commit is contained in:
davi@mysql.com/endora.local 2007-11-29 09:42:26 -02:00
parent de0175204d
commit d179bb64c2
10 changed files with 207 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
mysql-test/r/trigger.result
View File

@ -1981,4 +1981,39 @@ drop table table_25411_b;
DROP TRIGGER IF EXISTS trg;
SHOW CREATE TRIGGER trg;
ERROR HY000: Trigger does not exist
drop table if exists t1;
create table t1 (i int, j int);
create trigger t1_bi before insert on t1 for each row begin end;
create trigger t1_bi before insert on t1 for each row begin end;
ERROR 42000: This version of MySQL doesn't yet support 'multiple triggers with the same action time and event for one table'
drop trigger t1_bi;
drop trigger t1_bi;
ERROR HY000: Trigger does not exist
lock tables t1 read;
create trigger t1_bi before insert on t1 for each row begin end;
ERROR HY000: Table 't1' was locked with a READ lock and can't be updated
create trigger t1_bi before insert on t1 for each row begin end;
ERROR HY000: Table 't1' was locked with a READ lock and can't be updated
drop trigger t1_bi;
ERROR HY000: Trigger does not exist
unlock tables;
create trigger t1_bi before insert on t1 for each row begin end;
lock tables t1 read;
create trigger t1_bi before insert on t1 for each row begin end;
ERROR HY000: Table 't1' was locked with a READ lock and can't be updated
drop trigger t1_bi;
ERROR HY000: Table 't1' was locked with a READ lock and can't be updated
unlock tables;
drop trigger t1_bi;
lock tables t1 write;
create trigger b1_bi before insert on t1 for each row set new.i = new.i + 10;
insert into t1 values (10, 10);
drop trigger b1_bi;
insert into t1 values (10, 10);
select * from t1;
i j
20 10
10 10
unlock tables;
drop table t1;
End of 5.1 tests.

1
mysql-test/r/trigger-grant.result → mysql-test/r/trigger_notembedded.result
View File

@ -448,3 +448,4 @@ DROP TABLE t1;
DROP DATABASE mysqltest_db1;
USE test;
End of 5.0 tests.
End of 5.1 tests.

47
mysql-test/t/trigger.test
View File

@ -2257,4 +2257,51 @@ DROP TRIGGER IF EXISTS trg;
--error ER_TRG_DOES_NOT_EXIST
SHOW CREATE TRIGGER trg;
#
# Bug#23713 LOCK TABLES + CREATE TRIGGER + FLUSH TABLES WITH READ LOCK = deadlock
#
# Test of trigger creation and removal under LOCK TABLES
#
--disable_warnings
drop table if exists t1;
--enable_warnings
create table t1 (i int, j int);
create trigger t1_bi before insert on t1 for each row begin end;
--error ER_NOT_SUPPORTED_YET
create trigger t1_bi before insert on t1 for each row begin end;
drop trigger t1_bi;
--error ER_TRG_DOES_NOT_EXIST
drop trigger t1_bi;
lock tables t1 read;
--error ER_TABLE_NOT_LOCKED_FOR_WRITE
create trigger t1_bi before insert on t1 for each row begin end;
--error ER_TABLE_NOT_LOCKED_FOR_WRITE
create trigger t1_bi before insert on t1 for each row begin end;
--error ER_TRG_DOES_NOT_EXIST
drop trigger t1_bi;
unlock tables;
create trigger t1_bi before insert on t1 for each row begin end;
lock tables t1 read;
--error ER_TABLE_NOT_LOCKED_FOR_WRITE
create trigger t1_bi before insert on t1 for each row begin end;
--error ER_TABLE_NOT_LOCKED_FOR_WRITE
drop trigger t1_bi;
unlock tables;
drop trigger t1_bi;
lock tables t1 write;
create trigger b1_bi before insert on t1 for each row set new.i = new.i + 10;
insert into t1 values (10, 10);
drop trigger b1_bi;
insert into t1 values (10, 10);
select * from t1;
unlock tables;
drop table t1;
--echo End of 5.1 tests.

34
mysql-test/t/trigger-grant.test → mysql-test/t/trigger_notembedded.test
View File

@ -875,3 +875,37 @@ DROP DATABASE mysqltest_db1;
USE test;
--echo End of 5.0 tests.
#
# Bug#23713 LOCK TABLES + CREATE TRIGGER + FLUSH TABLES WITH READ LOCK = deadlock
#
# Test temporarily disable due to Bug#32395
--disable_parsing
create table t1 (i int);
connect (flush,localhost,root,,test,,);
connection default;
--echo connection: default
lock tables t1 write;
connection flush;
--echo connection: flush
--send flush tables with read lock;
connection default;
--echo connection: default
let $wait_condition=
select count(*) = 1 from information_schema.processlist
where state = "Flushing tables";
--source include/wait_condition.inc
create trigger t1_bi before insert on t1 for each row begin end;
unlock tables;
connection flush;
--echo connection: flush
--reap
unlock tables;
connection default;
select * from t1;
drop table t1;
disconnect flush;
--enable_parsing
--echo End of 5.1 tests.

6
sql/mysql_priv.h
View File

@ -978,7 +978,8 @@ bool check_dup(const char *db, const char *name, TABLE_LIST *tables);
bool compare_record(TABLE *table);
bool append_file_to_dir(THD *thd, const char **filename_ptr,
const char *table_name);
void wait_while_table_is_used(THD *thd, TABLE *table,
enum ha_extra_function function);
bool table_cache_init(void);
void table_cache_free(void);
bool table_def_init(void);
@ -1141,6 +1142,7 @@ TABLE *open_ltable(THD *thd, TABLE_LIST *table_list, thr_lock_type update,
uint lock_flags);
TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT* mem,
bool *refresh, uint flags);
bool name_lock_locked_table(THD *thd, TABLE_LIST *tables);
bool reopen_name_locked_table(THD* thd, TABLE_LIST* table_list, bool link_in);
TABLE *table_cache_insert_placeholder(THD *thd, const char *key,
uint key_length);
@ -1292,7 +1294,7 @@ bool mysql_ha_close(THD *thd, TABLE_LIST *tables);
bool mysql_ha_read(THD *, TABLE_LIST *,enum enum_ha_read_modes,char *,
List<Item> *,enum ha_rkey_function,Item *,ha_rows,ha_rows);
void mysql_ha_flush(THD *thd);
void mysql_ha_rm_tables(THD *thd, TABLE_LIST *tables);
void mysql_ha_rm_tables(THD *thd, TABLE_LIST *tables, bool is_locked);
void mysql_ha_cleanup(THD *thd);
/* sql_base.cc */

42
sql/sql_base.cc
View File

@ -2199,6 +2199,41 @@ void wait_for_condition(THD *thd, pthread_mutex_t *mutex, pthread_cond_t *cond)
}
/**
Exclusively name-lock a table that is already write-locked by the
current thread.
@param thd current thread context
@param tables able list containing one table to open.
@return FALSE on success, TRUE otherwise.
*/
bool name_lock_locked_table(THD *thd, TABLE_LIST *tables)
{
DBUG_ENTER("name_lock_locked_table");
/* Under LOCK TABLES we must only accept write locked tables. */
tables->table= find_locked_table(thd, tables->db, tables->table_name);
if (!tables->table)
my_error(ER_TABLE_NOT_LOCKED, MYF(0), tables->alias);
else if (tables->table->reginfo.lock_type < TL_WRITE_LOW_PRIORITY)
my_error(ER_TABLE_NOT_LOCKED_FOR_WRITE, MYF(0), tables->alias);
else
{
/*
Ensures that table is opened only by this thread and that no
other statement will open this table.
*/
wait_while_table_is_used(thd, tables->table, HA_EXTRA_FORCE_REOPEN);
DBUG_RETURN(FALSE);
}
DBUG_RETURN(TRUE);
}
/*
Open table which is already name-locked by this thread.
@ -3118,6 +3153,9 @@ bool reopen_table(TABLE *table)
then there is only one table open and locked. This means that
the function probably has to be adjusted before it can be used
anywhere outside ALTER TABLE.
@note Must not use TABLE_SHARE::table_name/db of the table being closed,
the strings are used in a loop even after the share may be freed.
*/
void close_data_files_and_morph_locks(THD *thd, const char *db,
@ -3387,8 +3425,8 @@ bool reopen_tables(THD *thd,bool get_locks,bool in_refresh)
@param send_refresh Should we awake waiters even if we didn't close any tables?
*/
void close_old_data_files(THD *thd, TABLE *table, bool morph_locks,
bool send_refresh)
static void close_old_data_files(THD *thd, TABLE *table, bool morph_locks,
bool send_refresh)
{
bool found= send_refresh;
DBUG_ENTER("close_old_data_files");

7
sql/sql_handler.cc
View File

@ -714,17 +714,16 @@ static TABLE_LIST *mysql_ha_find(THD *thd, TABLE_LIST *tables)
@param thd Thread identifier.
@param tables The list of tables to remove.
@param is_locked If LOCK_open is locked.
@note Broadcasts refresh if it closed a table with old version.
*/
void mysql_ha_rm_tables(THD *thd, TABLE_LIST *tables)
void mysql_ha_rm_tables(THD *thd, TABLE_LIST *tables, bool is_locked)
{
TABLE_LIST *hash_tables, *next;
DBUG_ENTER("mysql_ha_rm_tables");
safe_mutex_assert_not_owner(&LOCK_open);
DBUG_ASSERT(tables);
hash_tables= mysql_ha_find(thd, tables);
@ -733,7 +732,7 @@ void mysql_ha_rm_tables(THD *thd, TABLE_LIST *tables)
{
next= hash_tables->next_local;
if (hash_tables->table)
mysql_ha_close_table(thd, hash_tables, FALSE);
mysql_ha_close_table(thd, hash_tables, is_locked);
hash_delete(&thd->handler_tables_hash, (uchar*) hash_tables);
hash_tables= next;
}

2
sql/sql_rename.cc
View File

@ -51,7 +51,7 @@ bool mysql_rename_tables(THD *thd, TABLE_LIST *table_list, bool silent)
DBUG_RETURN(1);
}
mysql_ha_rm_tables(thd, table_list);
mysql_ha_rm_tables(thd, table_list, FALSE);
if (wait_if_global_read_lock(thd,0,1))
DBUG_RETURN(1);

12
sql/sql_table.cc
View File

@ -1521,7 +1521,7 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
built_query.append("DROP TABLE ");
}
mysql_ha_rm_tables(thd, tables);
mysql_ha_rm_tables(thd, tables, FALSE);
pthread_mutex_lock(&LOCK_open);
@ -3705,14 +3705,16 @@ mysql_rename_table(handlerton *base, const char *old_db,
Win32 clients must also have a WRITE LOCK on the table !
*/
static void wait_while_table_is_used(THD *thd,TABLE *table,
enum ha_extra_function function)
void wait_while_table_is_used(THD *thd, TABLE *table,
enum ha_extra_function function)
{
DBUG_ENTER("wait_while_table_is_used");
DBUG_PRINT("enter", ("table: '%s' share: 0x%lx db_stat: %u version: %lu",
table->s->table_name.str, (ulong) table->s,
table->db_stat, table->s->version));
safe_mutex_assert_owner(&LOCK_open);
VOID(table->file->extra(function));
/* Mark all tables that are in use as 'old' */
mysql_lock_abort(thd, table, TRUE); /* end threads waiting on lock */
@ -4031,7 +4033,7 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables,
Protocol::SEND_NUM_ROWS | Protocol::SEND_EOF))
DBUG_RETURN(TRUE);
mysql_ha_rm_tables(thd, tables);
mysql_ha_rm_tables(thd, tables, FALSE);
for (table= tables; table; table= table->next_local)
{
@ -5795,7 +5797,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
build_table_filename(reg_path, sizeof(reg_path), db, table_name, reg_ext, 0);
build_table_filename(path, sizeof(path), db, table_name, "", 0);
mysql_ha_rm_tables(thd, table_list);
mysql_ha_rm_tables(thd, table_list, FALSE);
/* DISCARD/IMPORT TABLESPACE is always alone in an ALTER TABLE */
if (alter_info->tablespace_op != NO_TABLESPACE_OP)

68
sql/sql_trigger.cc
View File

@ -323,6 +323,7 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create)
TABLE *table;
bool result= TRUE;
String stmt_query;
bool need_start_waiting= FALSE;
DBUG_ENTER("mysql_create_or_drop_trigger");
@ -374,10 +375,12 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create)
/*
We don't want perform our operations while global read lock is held
so we have to wait until its end and then prevent it from occurring
again until we are done. (Acquiring LOCK_open is not enough because
global read lock is held without holding LOCK_open).
again until we are done, unless we are under lock tables. (Acquiring
LOCK_open is not enough because global read lock is held without holding
LOCK_open).
*/
if (wait_if_global_read_lock(thd, 0, 1))
if (!thd->locked_tables &&
!(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1)))
DBUG_RETURN(TRUE);
VOID(pthread_mutex_lock(&LOCK_open));
@ -433,35 +436,25 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create)
goto end;
}
if (lock_table_names(thd, tables))
goto end;
/* We also don't allow creation of triggers on views. */
tables->required_type= FRMTYPE_TABLE;
/*
If the table is under LOCK TABLES, lock_table_names() does not set
tables->table. Find the table in open_tables.
*/
if (!tables->table && thd->locked_tables)
/* Keep consistent with respect to other DDL statements */
mysql_ha_rm_tables(thd, tables, TRUE);
if (thd->locked_tables)
{
for (table= thd->open_tables;
table && (strcmp(table->s->table_name.str, tables->table_name) ||
strcmp(table->s->db.str, tables->db));
table= table->next) {}
tables->table= table;
/* Table must be write locked */
if (name_lock_locked_table(thd, tables))
goto end;
}
if (!tables->table)
else
{
/* purecov: begin inspected */
my_error(ER_TABLE_NOT_LOCKED, MYF(0), tables->alias);
goto end;
/* purecov: end */
}
/* No need to reopen the table if it is locked with LOCK TABLES. */
if (!thd->locked_tables || (tables->table->in_use != thd))
{
/* We also don't allow creation of triggers on views. */
tables->required_type= FRMTYPE_TABLE;
/* Grab the name lock and insert the placeholder*/
if (lock_table_names(thd, tables))
goto end;
/* Convert the placeholder to a real table */
if (reopen_name_locked_table(thd, tables, TRUE))
{
unlock_table_name(thd, tables);
@ -489,13 +482,20 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create)
/* Under LOCK TABLES we must reopen the table to activate the trigger. */
if (!result && thd->locked_tables)
{
/*
Must not use table->s->db.str or table->s->table_name.str here.
The strings are used in a loop even after the share may be freed.
*/
/* Make table suitable for reopening */
close_data_files_and_morph_locks(thd, tables->db, tables->table_name);
thd->in_lock_tables= 1;
result= reopen_tables(thd, 1, 0);
if (reopen_tables(thd, 1, 1))
{
/* To be safe remove this table from the set of LOCKED TABLES */
unlink_open_table(thd, tables->table, FALSE);
/*
Ignore reopen_tables errors for now. It's better not leave master/slave
in a inconsistent state.
*/
thd->clear_error();
}
thd->in_lock_tables= 0;
}
@ -507,7 +507,9 @@ end:
}
VOID(pthread_mutex_unlock(&LOCK_open));
start_waiting_global_read_lock(thd);
if (need_start_waiting)
start_waiting_global_read_lock(thd);
if (!result)
send_ok(thd);