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

Fix for bug #12739 "Deadlock in multithreaded environment during creating/

Browse Source 
droping trigger on InnoDB table".

Deadlock occured in cases when we were trying to create two triggers for
the same InnoDB table concurrently and both threads were able to reach
close_cached_table() simultaneously. Bugfix implements new approach to
table locking and table cache invalidation during creation/dropping
of trigger.

No testcase is supplied since bug was repeatable only under high concurrency.
This commit is contained in:
dlenev@mysql.com 2005-10-17 22:37:24 +04:00
parent dad1e2048d
commit ea0b89aeae
4 changed files with 99 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
sql/mysql_priv.h
View File

@ -757,7 +757,7 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create);
TABLE *open_ltable(THD *thd, TABLE_LIST *table_list, thr_lock_type update);
TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT* mem,
bool *refresh, uint flags);
TABLE *reopen_name_locked_table(THD* thd, TABLE_LIST* table);
bool reopen_name_locked_table(THD* thd, TABLE_LIST* table);
TABLE *find_locked_table(THD *thd, const char *db,const char *table_name);
bool reopen_table(TABLE *table,bool locked);
bool reopen_tables(THD *thd,bool get_locks,bool in_refresh);

64
sql/sql_base.cc
View File

@ -976,32 +976,57 @@ void wait_for_refresh(THD *thd)
}
TABLE *reopen_name_locked_table(THD* thd, TABLE_LIST* table_list)
{
DBUG_ENTER("reopen_name_locked_table");
if (thd->killed)
DBUG_RETURN(0);
TABLE *table;
TABLE_SHARE *share;
if (!(table = table_list->table))
DBUG_RETURN(0);
/*
Open table which is already name-locked by this thread.
char* db = thd->db ? thd->db : table_list->db;
char* table_name = table_list->table_name;
char key[MAX_DBKEY_LENGTH];
uint key_length;
SYNOPSIS
reopen_name_locked_table()
thd Thread handle
table_list TABLE_LIST object for table to be open, TABLE_LIST::table
member should point to TABLE object which was used for
name-locking.
NOTE
This function assumes that its caller already acquired LOCK_open mutex.
RETURN VALUE
FALSE - Success
TRUE - Error
*/
bool reopen_name_locked_table(THD* thd, TABLE_LIST* table_list)
{
TABLE *table= table_list->table;
TABLE_SHARE *share;
char *db= table_list->db;
char *table_name= table_list->table_name;
char key[MAX_DBKEY_LENGTH];
uint key_length;
TABLE orig_table;
DBUG_ENTER("reopen_name_locked_table");
safe_mutex_assert_owner(&LOCK_open);
if (thd->killed || !table)
DBUG_RETURN(TRUE);
orig_table= *table;
key_length=(uint) (strmov(strmov(key,db)+1,table_name)-key)+1;
pthread_mutex_lock(&LOCK_open);
if (open_unireg_entry(thd, table, db, table_name, table_name, 0,
thd->mem_root) ||
!(table->s->table_cache_key= memdup_root(&table->mem_root, (char*) key,
key_length)))
{
delete table->triggers;
closefrm(table);
pthread_mutex_unlock(&LOCK_open);
DBUG_RETURN(0);
intern_close_table(table);
/*
If there was an error during opening of table (for example if it
does not exist) '*table' object can be wiped out. To be able
properly release name-lock in this case we should restore this
object to its original state.
*/
*table= orig_table;
DBUG_RETURN(TRUE);
}
share= table->s;
@ -1011,7 +1036,6 @@ TABLE *reopen_name_locked_table(THD* thd, TABLE_LIST* table_list)
share->flush_version=0;
table->in_use = thd;
check_unused();
pthread_mutex_unlock(&LOCK_open);
table->next = thd->open_tables;
thd->open_tables = table;
table->tablenr=thd->current_tablenr++;
@ -1021,7 +1045,7 @@ TABLE *reopen_name_locked_table(THD* thd, TABLE_LIST* table_list)
table->status=STATUS_NO_RECORD;
table->keys_in_use_for_query= share->keys_in_use;
table->used_keys= share->keys_for_keyread;
DBUG_RETURN(table);
DBUG_RETURN(FALSE);
}

19
sql/sql_table.cc
View File

@ -1950,8 +1950,8 @@ static int prepare_for_restore(THD* thd, TABLE_LIST* table,
{
char* backup_dir= thd->lex->backup_dir;
char src_path[FN_REFLEN], dst_path[FN_REFLEN];
char* table_name = table->table_name;
char* db = thd->db ? thd->db : table->db;
char* table_name= table->table_name;
char* db= table->db;
if (fn_format_relative_to_data_home(src_path, table_name, backup_dir,
reg_ext))
@ -1987,12 +1987,15 @@ static int prepare_for_restore(THD* thd, TABLE_LIST* table,
Now we should be able to open the partially restored table
to finish the restore in the handler later on
*/
if (!(table->table = reopen_name_locked_table(thd, table)))
pthread_mutex_lock(&LOCK_open);
if (reopen_name_locked_table(thd, table))
{
pthread_mutex_lock(&LOCK_open);
unlock_table_name(thd, table);
pthread_mutex_unlock(&LOCK_open);
DBUG_RETURN(send_check_errmsg(thd, table, "restore",
"Failed to open partially restored table"));
}
pthread_mutex_unlock(&LOCK_open);
DBUG_RETURN(0);
}
@ -2089,12 +2092,16 @@ static int prepare_for_repair(THD* thd, TABLE_LIST *table_list,
Now we should be able to open the partially repaired table
to finish the repair in the handler later on.
*/
if (!(table_list->table = reopen_name_locked_table(thd, table_list)))
pthread_mutex_lock(&LOCK_open);
if (reopen_name_locked_table(thd, table_list))
{
pthread_mutex_lock(&LOCK_open);
unlock_table_name(thd, table_list);
pthread_mutex_unlock(&LOCK_open);
error= send_check_errmsg(thd, table_list, "repair",
"Failed to open partially repaired table");
goto end;
}
pthread_mutex_unlock(&LOCK_open);
end:
if (table == &tmp_table)

80
sql/sql_trigger.cc
View File

@ -103,8 +103,7 @@ static TABLE_LIST *add_table_for_trigger(THD *thd, sp_name *trig);
bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create)
{
TABLE *table;
bool result= 0;
bool result= TRUE;
DBUG_ENTER("mysql_create_or_drop_trigger");
/*
@ -119,9 +118,6 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create)
/* We should have only one table in table list. */
DBUG_ASSERT(tables->next_global == 0);
if (!(table= open_ltable(thd, tables, tables->lock_type)))
DBUG_RETURN(TRUE);
/*
TODO: We should check if user has TRIGGER privilege for table here.
Now we just require SUPER privilege for creating/dropping because
@ -131,28 +127,24 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create)
DBUG_RETURN(TRUE);
/*
We do not allow creation of triggers on temporary tables. We also don't
allow creation of triggers on views but fulfilment of this restriction
is guaranteed by open_ltable(). It is better to have this check here
than do it in Table_triggers_list::create_trigger() and mess with table
cache.
There is no DETERMINISTIC clause for triggers, so can't check it.
But a trigger can in theory be used to do nasty things (if it supported
DROP for example) so we do the check for privileges. For now there is
already a stronger test right above; but when this stronger test will
be removed, the test below will hold.
*/
if (table->s->tmp_table != NO_TMP_TABLE)
if (!trust_routine_creators && mysql_bin_log.is_open() &&
!(thd->security_ctx->master_access & SUPER_ACL))
{
my_error(ER_TRG_ON_VIEW_OR_TEMP_TABLE, MYF(0), tables->alias);
my_error(ER_BINLOG_CREATE_ROUTINE_NEED_SUPER, MYF(0));
DBUG_RETURN(TRUE);
}
if (!table->triggers)
/* We do not allow creation of triggers on temporary tables. */
if (create && find_temporary_table(thd, tables->db, tables->table_name))
{
if (!create)
{
my_message(ER_TRG_DOES_NOT_EXIST, ER(ER_TRG_DOES_NOT_EXIST), MYF(0));
DBUG_RETURN(TRUE);
}
if (!(table->triggers= new (&table->mem_root) Table_triggers_list(table)))
DBUG_RETURN(TRUE);
my_error(ER_TRG_ON_VIEW_OR_TEMP_TABLE, MYF(0), tables->alias);
DBUG_RETURN(TRUE);
}
/*
@ -161,31 +153,41 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create)
again until we are done. (Acquiring LOCK_open is not enough because
global read lock is held without helding LOCK_open).
*/
if (wait_if_global_read_lock(thd, 0, 0))
if (wait_if_global_read_lock(thd, 0, 1))
DBUG_RETURN(TRUE);
/*
There is no DETERMINISTIC clause for triggers, so can't check it.
But a trigger can in theory be used to do nasty things (if it supported
DROP for example) so we do the check for privileges. For now there is
already a stronger test above (see start of the function); but when this
stronger test will be removed, the test below will hold.
*/
if (!trust_routine_creators && mysql_bin_log.is_open() &&
!(thd->security_ctx->master_access & SUPER_ACL))
{
my_message(ER_BINLOG_CREATE_ROUTINE_NEED_SUPER,
ER(ER_BINLOG_CREATE_ROUTINE_NEED_SUPER), MYF(0));
DBUG_RETURN(TRUE);
}
VOID(pthread_mutex_lock(&LOCK_open));
if (lock_table_names(thd, tables))
goto end;
/* We also don't allow creation of triggers on views. */
tables->required_type= FRMTYPE_TABLE;
if (reopen_name_locked_table(thd, tables))
{
unlock_table_name(thd, tables);
goto end;
}
table= tables->table;
if (!table->triggers)
{
if (!create)
{
my_error(ER_TRG_DOES_NOT_EXIST, MYF(0));
goto end;
}
if (!(table->triggers= new (&table->mem_root) Table_triggers_list(table)))
goto end;
}
result= (create ?
table->triggers->create_trigger(thd, tables):
table->triggers->drop_trigger(thd, tables));
/* It is sensible to invalidate table in any case */
close_cached_table(thd, table);
end:
VOID(pthread_mutex_unlock(&LOCK_open));
start_waiting_global_read_lock(thd);