From bca6da5434b9124b2fb3bc0577f6e43de437f2e3 Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 19 Jul 2005 00:29:19 +0200 Subject: Bug #10600 remove_table_from_cache fails to signal other thread and gets blocked when other thread also gets blocked include/thr_lock.h: Report if any threads was signalled mysys/thr_lock.c: Report if any threads was signalled sql/lock.cc: Report if any threads was signalled Use new interface for remove_table_from_cache sql/mysql_priv.h: New interface for remove_table_from_cache + mysql_lock_abort_for_thread sql/sql_base.cc: Use new interface of remove_table_cache Rewrote remove_table_from_cache to fix bug sql/sql_table.cc: Use new interface of remove_table_from_cache --- include/thr_lock.h | 2 +- mysys/thr_lock.c | 7 ++- sql/lock.cc | 24 +++++++--- sql/mysql_priv.h | 7 ++- sql/sql_base.cc | 132 ++++++++++++++++++++++++++++++++++++----------------- sql/sql_table.cc | 38 ++++++--------- 6 files changed, 133 insertions(+), 77 deletions(-) diff --git a/include/thr_lock.h b/include/thr_lock.h index f1bda0ce6b4..8caaa112605 100644 --- a/include/thr_lock.h +++ b/include/thr_lock.h @@ -107,7 +107,7 @@ void thr_unlock(THR_LOCK_DATA *data); int thr_multi_lock(THR_LOCK_DATA **data,uint count); void thr_multi_unlock(THR_LOCK_DATA **data,uint count); void thr_abort_locks(THR_LOCK *lock); -void thr_abort_locks_for_thread(THR_LOCK *lock, pthread_t thread); +bool thr_abort_locks_for_thread(THR_LOCK *lock, pthread_t thread); void thr_print_locks(void); /* For debugging */ my_bool thr_upgrade_write_delay_lock(THR_LOCK_DATA *data); my_bool thr_reschedule_write_lock(THR_LOCK_DATA *data); diff --git a/mysys/thr_lock.c b/mysys/thr_lock.c index 935ed4ea282..63dbe67c68e 100644 --- a/mysys/thr_lock.c +++ b/mysys/thr_lock.c @@ -966,9 +966,10 @@ void thr_abort_locks(THR_LOCK *lock) This is used to abort all locks for a specific thread */ -void thr_abort_locks_for_thread(THR_LOCK *lock, pthread_t thread) +bool thr_abort_locks_for_thread(THR_LOCK *lock, pthread_t thread) { THR_LOCK_DATA *data; + bool found= FALSE; DBUG_ENTER("thr_abort_locks_for_thread"); pthread_mutex_lock(&lock->mutex); @@ -978,6 +979,7 @@ void thr_abort_locks_for_thread(THR_LOCK *lock, pthread_t thread) { DBUG_PRINT("info",("Aborting read-wait lock")); data->type= TL_UNLOCK; /* Mark killed */ + found= TRUE; pthread_cond_signal(data->cond); data->cond= 0; /* Removed from list */ @@ -993,6 +995,7 @@ void thr_abort_locks_for_thread(THR_LOCK *lock, pthread_t thread) { DBUG_PRINT("info",("Aborting write-wait lock")); data->type= TL_UNLOCK; + found= TRUE; pthread_cond_signal(data->cond); data->cond= 0; @@ -1003,7 +1006,7 @@ void thr_abort_locks_for_thread(THR_LOCK *lock, pthread_t thread) } } pthread_mutex_unlock(&lock->mutex); - DBUG_VOID_RETURN; + DBUG_RETURN(found); } diff --git a/sql/lock.cc b/sql/lock.cc index 4c6d7c75ae2..dbd7748e104 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -333,20 +333,25 @@ void mysql_lock_abort(THD *thd, TABLE *table) /* Abort one thread / table combination */ -void mysql_lock_abort_for_thread(THD *thd, TABLE *table) +bool mysql_lock_abort_for_thread(THD *thd, TABLE *table) { MYSQL_LOCK *locked; TABLE *write_lock_used; + bool result= FALSE; DBUG_ENTER("mysql_lock_abort_for_thread"); if ((locked = get_lock_data(thd,&table,1,1,&write_lock_used))) { for (uint i=0; i < locked->lock_count; i++) - thr_abort_locks_for_thread(locked->locks[i]->lock, - table->in_use->real_id); + { + bool found; + found= thr_abort_locks_for_thread(locked->locks[i]->lock, + table->in_use->real_id); + result|= found; + } my_free((gptr) locked,MYF(0)); } - DBUG_VOID_RETURN; + DBUG_RETURN(result); } @@ -542,8 +547,15 @@ int lock_table_name(THD *thd, TABLE_LIST *table_list) my_free((gptr) table,MYF(0)); DBUG_RETURN(-1); } - if (remove_table_from_cache(thd, db, table_list->real_name)) - DBUG_RETURN(1); // Table is in use + + { + uint flags= 0; + if (remove_table_from_cache(thd, db, + table_list->real_name, flags)) + { + DBUG_RETURN(1); // Table is in use + } + } DBUG_RETURN(0); } diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index 75ec3482d8d..d28a1d5763f 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -606,8 +606,11 @@ bool rename_temporary_table(THD* thd, TABLE *table, const char *new_db, const char *table_name); void remove_db_from_cache(const my_string db); void flush_tables(); +#define OWNED_BY_THD_FLAG 1 +#define WAIT_OTHER_THREAD_FLAG 2 +#define CHECK_KILLED_FLAG 4 bool remove_table_from_cache(THD *thd, const char *db, const char *table, - bool return_if_owned_by_thd=0); + uint flags); bool close_cached_tables(THD *thd, bool wait_for_refresh, TABLE_LIST *tables); void copy_field_from_tmp_record(Field *field,int offset); int fill_record(List &fields,List &values, bool ignore_errors); @@ -776,7 +779,7 @@ void mysql_unlock_read_tables(THD *thd, MYSQL_LOCK *sql_lock); void mysql_unlock_some_tables(THD *thd, TABLE **table,uint count); void mysql_lock_remove(THD *thd, MYSQL_LOCK *locked,TABLE *table); void mysql_lock_abort(THD *thd, TABLE *table); -void mysql_lock_abort_for_thread(THD *thd, TABLE *table); +bool mysql_lock_abort_for_thread(THD *thd, TABLE *table); MYSQL_LOCK *mysql_lock_merge(MYSQL_LOCK *a,MYSQL_LOCK *b); bool lock_global_read_lock(THD *thd); void unlock_global_read_lock(THD *thd); diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 9c5f73697fd..62b0bf05829 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -370,7 +370,9 @@ bool close_cached_tables(THD *thd, bool if_wait_for_refresh, bool found=0; for (TABLE_LIST *table=tables ; table ; table=table->next) { - if (remove_table_from_cache(thd, table->db, table->real_name, 1)) + uint flags= OWNED_BY_THD_FLAG; + if (remove_table_from_cache(thd, table->db, table->real_name, + flags)) found=1; } if (!found) @@ -2407,62 +2409,106 @@ void flush_tables() */ bool remove_table_from_cache(THD *thd, const char *db, const char *table_name, - bool return_if_owned_by_thd) + uint flags) { char key[MAX_DBKEY_LENGTH]; uint key_length; TABLE *table; - bool result=0; + bool result=0, signalled= 0; + bool return_if_owned_by_thd= flags & OWNED_BY_THD_FLAG; + bool wait_for_other_thread= flags & WAIT_OTHER_THREAD_FLAG; + bool check_killed_flag= flags & CHECK_KILLED_FLAG; DBUG_ENTER("remove_table_from_cache"); key_length=(uint) (strmov(strmov(key,db)+1,table_name)-key)+1; - for (table=(TABLE*) hash_search(&open_cache,(byte*) key,key_length) ; - table; - table = (TABLE*) hash_next(&open_cache,(byte*) key,key_length)) + do { - THD *in_use; - table->version=0L; /* Free when thread is ready */ - if (!(in_use=table->in_use)) - { - DBUG_PRINT("info",("Table was not in use")); - relink_unused(table); - } - else if (in_use != thd) + result= signalled= 0; + + for (table=(TABLE*) hash_search(&open_cache,(byte*) key,key_length) ; + table; + table = (TABLE*) hash_next(&open_cache,(byte*) key,key_length)) { - in_use->some_tables_deleted=1; - if (table->db_stat) - result=1; - /* Kill delayed insert threads */ - if ((in_use->system_thread & SYSTEM_THREAD_DELAYED_INSERT) && - ! in_use->killed) + THD *in_use; + table->version=0L; /* Free when thread is ready */ + if (!(in_use=table->in_use)) { - in_use->killed=1; - pthread_mutex_lock(&in_use->mysys_var->mutex); - if (in_use->mysys_var->current_cond) - { - pthread_mutex_lock(in_use->mysys_var->current_mutex); - pthread_cond_broadcast(in_use->mysys_var->current_cond); - pthread_mutex_unlock(in_use->mysys_var->current_mutex); - } - pthread_mutex_unlock(&in_use->mysys_var->mutex); + DBUG_PRINT("info",("Table was not in use")); + relink_unused(table); } - /* - Now we must abort all tables locks used by this thread - as the thread may be waiting to get a lock for another table - */ - for (TABLE *thd_table= in_use->open_tables; - thd_table ; - thd_table= thd_table->next) + else if (in_use != thd) + { + in_use->some_tables_deleted=1; + if (table->db_stat) + result=1; + /* Kill delayed insert threads */ + if ((in_use->system_thread & SYSTEM_THREAD_DELAYED_INSERT) && + ! in_use->killed) + { + in_use->killed=1; + pthread_mutex_lock(&in_use->mysys_var->mutex); + if (in_use->mysys_var->current_cond) + { + pthread_mutex_lock(in_use->mysys_var->current_mutex); + signalled= 1; + pthread_cond_broadcast(in_use->mysys_var->current_cond); + pthread_mutex_unlock(in_use->mysys_var->current_mutex); + } + pthread_mutex_unlock(&in_use->mysys_var->mutex); + } + /* + Now we must abort all tables locks used by this thread + as the thread may be waiting to get a lock for another table + */ + for (TABLE *thd_table= in_use->open_tables; + thd_table ; + thd_table= thd_table->next) + { + if (thd_table->db_stat) // If table is open + { + bool found; + found= mysql_lock_abort_for_thread(thd, thd_table); + signalled|= found; + } + } + } + else + result= result || return_if_owned_by_thd; + } + while (unused_tables && !unused_tables->version) + VOID(hash_delete(&open_cache,(byte*) unused_tables)); + if (result && wait_for_other_thread) + { + if (!check_killed_flag || !thd->killed) { - if (thd_table->db_stat) // If table is open - mysql_lock_abort_for_thread(thd, thd_table); + if (likely(signalled)) + { + dropping_tables++; + (void) pthread_cond_wait(&COND_refresh, &LOCK_open); + dropping_tables--; + continue; + } + else + { + /* + It can happen that another thread has opened the + table but has not yet locked any table at all. Since + it can be locked waiting for a table that our thread + has done LOCK TABLE x WRITE on previously, we need to + ensure that the thread actually hears our signal + before we go to sleep. Thus we wait for a short time + and then we retry another loop in the + remove_table_from_cache routine. + */ + pthread_mutex_unlock(&LOCK_open); + my_sleep(10); + pthread_mutex_lock(&LOCK_open); + continue; + } } } - else - result= result || return_if_owned_by_thd; - } - while (unused_tables && !unused_tables->version) - VOID(hash_delete(&open_cache,(byte*) unused_tables)); + break; + } while (1); DBUG_RETURN(result); } diff --git a/sql/sql_table.cc b/sql/sql_table.cc index a9af8144df0..d0ef29cee50 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -178,6 +178,7 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, for (table=tables ; table ; table=table->next) { char *db=table->db; + uint flags; mysql_ha_flush(thd, table, MYSQL_HA_CLOSE_FINAL); if (!close_temporary_table(thd, db, table->real_name)) { @@ -186,12 +187,8 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, } abort_locked_tables(thd,db,table->real_name); - while (remove_table_from_cache(thd,db,table->real_name) && !thd->killed) - { - dropping_tables++; - (void) pthread_cond_wait(&COND_refresh,&LOCK_open); - dropping_tables--; - } + flags= WAIT_OTHER_THREAD_FLAG | CHECK_KILLED_FLAG; + remove_table_from_cache(thd,db,table->real_name,flags); drop_locked_tables(thd,db,table->real_name); if (thd->killed) DBUG_RETURN(-1); @@ -979,6 +976,7 @@ mysql_rename_table(enum db_type base, static void wait_while_table_is_used(THD *thd,TABLE *table, enum ha_extra_function function) { + uint flags; DBUG_PRINT("enter",("table: %s", table->real_name)); DBUG_ENTER("wait_while_table_is_used"); safe_mutex_assert_owner(&LOCK_open); @@ -988,13 +986,9 @@ static void wait_while_table_is_used(THD *thd,TABLE *table, mysql_lock_abort(thd, table); // end threads waiting on lock /* Wait until all there are no other threads that has this table open */ - while (remove_table_from_cache(thd,table->table_cache_key, - table->real_name)) - { - dropping_tables++; - (void) pthread_cond_wait(&COND_refresh,&LOCK_open); - dropping_tables--; - } + flags= WAIT_OTHER_THREAD_FLAG; + remove_table_from_cache(thd,table->table_cache_key, + table->real_name,flags); DBUG_VOID_RETURN; } @@ -1306,18 +1300,14 @@ static int mysql_admin_table(THD* thd, TABLE_LIST* tables, /* Close all instances of the table to allow repair to rename files */ if (lock_type == TL_WRITE && table->table->version) { + uint flags; pthread_mutex_lock(&LOCK_open); const char *old_message=thd->enter_cond(&COND_refresh, &LOCK_open, "Waiting to get writelock"); mysql_lock_abort(thd,table->table); - while (remove_table_from_cache(thd, table->table->table_cache_key, - table->table->real_name) && - ! thd->killed) - { - dropping_tables++; - (void) pthread_cond_wait(&COND_refresh,&LOCK_open); - dropping_tables--; - } + flags= WAIT_OTHER_THREAD_FLAG | CHECK_KILLED_FLAG; + remove_table_from_cache(thd, table->table->table_cache_key, + table->table->real_name, flags); thd->exit_cond(old_message); if (thd->killed) goto err; @@ -1382,9 +1372,10 @@ static int mysql_admin_table(THD* thd, TABLE_LIST* tables, table->table->version=0; // Force close of table else if (open_for_modify) { + uint flags= 0; pthread_mutex_lock(&LOCK_open); remove_table_from_cache(thd, table->table->table_cache_key, - table->table->real_name); + table->table->real_name, flags); pthread_mutex_unlock(&LOCK_open); /* May be something modified consequently we have to invalidate cache */ query_cache_invalidate3(thd, table->table, 0); @@ -2108,8 +2099,9 @@ int mysql_alter_table(THD *thd,char *new_db, char *new_name, */ if (table) { + uint flags= 0; VOID(table->file->extra(HA_EXTRA_FORCE_REOPEN)); // Use new file - remove_table_from_cache(thd,db,table_name); // Mark all in-use copies old + remove_table_from_cache(thd,db,table_name,flags);// Mark in-use copies old mysql_lock_abort(thd,table); // end threads waiting on lock } VOID(quick_rm_table(old_db_type,db,old_name)); -- cgit v1.2.1 From 1b6ac530667d8fa973da6a41755dcc1cfb84605d Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 20 Jul 2005 21:19:01 +0200 Subject: Bug #10600 After review fixes sql/lock.cc: Used flags immediately in call sql/mysql_priv.h: Added RTFC (short for remove_table_from_cache) for constants and used hex syntax to clarify it is bits in the flags sql/sql_base.cc: Use flags parameter immediately and use flags immediately in call Change to other variant of eternal loop variant sql/sql_table.cc: Use flags immediately in call --- sql/lock.cc | 3 +-- sql/mysql_priv.h | 7 ++++--- sql/sql_base.cc | 22 +++++++--------------- sql/sql_table.cc | 15 ++++++--------- 4 files changed, 18 insertions(+), 29 deletions(-) diff --git a/sql/lock.cc b/sql/lock.cc index dbd7748e104..eb49254215d 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -549,9 +549,8 @@ int lock_table_name(THD *thd, TABLE_LIST *table_list) } { - uint flags= 0; if (remove_table_from_cache(thd, db, - table_list->real_name, flags)) + table_list->real_name, RTFC_NO_FLAG)) { DBUG_RETURN(1); // Table is in use } diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index d28a1d5763f..bc7ec53ccd9 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -606,9 +606,10 @@ bool rename_temporary_table(THD* thd, TABLE *table, const char *new_db, const char *table_name); void remove_db_from_cache(const my_string db); void flush_tables(); -#define OWNED_BY_THD_FLAG 1 -#define WAIT_OTHER_THREAD_FLAG 2 -#define CHECK_KILLED_FLAG 4 +#define RTFC_NO_FLAG 0x0000 +#define RTFC_OWNED_BY_THD_FLAG 0x0001 +#define RTFC_WAIT_OTHER_THREAD_FLAG 0x0002 +#define RTFC_CHECK_KILLED_FLAG 0x0004 bool remove_table_from_cache(THD *thd, const char *db, const char *table, uint flags); bool close_cached_tables(THD *thd, bool wait_for_refresh, TABLE_LIST *tables); diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 62b0bf05829..32d14854e08 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -370,9 +370,8 @@ bool close_cached_tables(THD *thd, bool if_wait_for_refresh, bool found=0; for (TABLE_LIST *table=tables ; table ; table=table->next) { - uint flags= OWNED_BY_THD_FLAG; if (remove_table_from_cache(thd, table->db, table->real_name, - flags)) + RTFC_OWNED_BY_THD_FLAG)) found=1; } if (!found) @@ -2415,13 +2414,10 @@ bool remove_table_from_cache(THD *thd, const char *db, const char *table_name, uint key_length; TABLE *table; bool result=0, signalled= 0; - bool return_if_owned_by_thd= flags & OWNED_BY_THD_FLAG; - bool wait_for_other_thread= flags & WAIT_OTHER_THREAD_FLAG; - bool check_killed_flag= flags & CHECK_KILLED_FLAG; DBUG_ENTER("remove_table_from_cache"); key_length=(uint) (strmov(strmov(key,db)+1,table_name)-key)+1; - do + for (;;) { result= signalled= 0; @@ -2465,21 +2461,17 @@ bool remove_table_from_cache(THD *thd, const char *db, const char *table_name, thd_table= thd_table->next) { if (thd_table->db_stat) // If table is open - { - bool found; - found= mysql_lock_abort_for_thread(thd, thd_table); - signalled|= found; - } + signalled|= mysql_lock_abort_for_thread(thd, thd_table); } } else - result= result || return_if_owned_by_thd; + result= result || (flags & RTFC_OWNED_BY_THD_FLAG); } while (unused_tables && !unused_tables->version) VOID(hash_delete(&open_cache,(byte*) unused_tables)); - if (result && wait_for_other_thread) + if (result && (flags & RTFC_WAIT_OTHER_THREAD_FLAG)) { - if (!check_killed_flag || !thd->killed) + if (!(flags & RTFC_CHECK_KILLED_FLAG) || !thd->killed) { if (likely(signalled)) { @@ -2508,7 +2500,7 @@ bool remove_table_from_cache(THD *thd, const char *db, const char *table_name, } } break; - } while (1); + } DBUG_RETURN(result); } diff --git a/sql/sql_table.cc b/sql/sql_table.cc index d0ef29cee50..4c269e6830f 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -187,7 +187,7 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, } abort_locked_tables(thd,db,table->real_name); - flags= WAIT_OTHER_THREAD_FLAG | CHECK_KILLED_FLAG; + flags= RTFC_WAIT_OTHER_THREAD_FLAG | RTFC_CHECK_KILLED_FLAG; remove_table_from_cache(thd,db,table->real_name,flags); drop_locked_tables(thd,db,table->real_name); if (thd->killed) @@ -976,7 +976,6 @@ mysql_rename_table(enum db_type base, static void wait_while_table_is_used(THD *thd,TABLE *table, enum ha_extra_function function) { - uint flags; DBUG_PRINT("enter",("table: %s", table->real_name)); DBUG_ENTER("wait_while_table_is_used"); safe_mutex_assert_owner(&LOCK_open); @@ -986,9 +985,8 @@ static void wait_while_table_is_used(THD *thd,TABLE *table, mysql_lock_abort(thd, table); // end threads waiting on lock /* Wait until all there are no other threads that has this table open */ - flags= WAIT_OTHER_THREAD_FLAG; remove_table_from_cache(thd,table->table_cache_key, - table->real_name,flags); + table->real_name, RTFC_WAIT_OTHER_THREAD_FLAG); DBUG_VOID_RETURN; } @@ -1305,7 +1303,7 @@ static int mysql_admin_table(THD* thd, TABLE_LIST* tables, const char *old_message=thd->enter_cond(&COND_refresh, &LOCK_open, "Waiting to get writelock"); mysql_lock_abort(thd,table->table); - flags= WAIT_OTHER_THREAD_FLAG | CHECK_KILLED_FLAG; + flags= RTFC_WAIT_OTHER_THREAD_FLAG | RTFC_CHECK_KILLED_FLAG; remove_table_from_cache(thd, table->table->table_cache_key, table->table->real_name, flags); thd->exit_cond(old_message); @@ -1372,10 +1370,9 @@ static int mysql_admin_table(THD* thd, TABLE_LIST* tables, table->table->version=0; // Force close of table else if (open_for_modify) { - uint flags= 0; pthread_mutex_lock(&LOCK_open); remove_table_from_cache(thd, table->table->table_cache_key, - table->table->real_name, flags); + table->table->real_name, RTFC_NO_FLAG); pthread_mutex_unlock(&LOCK_open); /* May be something modified consequently we have to invalidate cache */ query_cache_invalidate3(thd, table->table, 0); @@ -2099,9 +2096,9 @@ int mysql_alter_table(THD *thd,char *new_db, char *new_name, */ if (table) { - uint flags= 0; VOID(table->file->extra(HA_EXTRA_FORCE_REOPEN)); // Use new file - remove_table_from_cache(thd,db,table_name,flags);// Mark in-use copies old + remove_table_from_cache(thd,db,table_name,RTFC_NO_FLAG); + // Mark in-use copies old mysql_lock_abort(thd,table); // end threads waiting on lock } VOID(quick_rm_table(old_db_type,db,old_name)); -- cgit v1.2.1