diff --git a/mysql-test/r/lock_multi.result b/mysql-test/r/lock_multi.result index f8cf539bd02..d7ba9174408 100644 --- a/mysql-test/r/lock_multi.result +++ b/mysql-test/r/lock_multi.result @@ -66,6 +66,21 @@ Select_priv N use test; use test; +CREATE TABLE t1 (c1 int); +LOCK TABLE t1 WRITE; + FLUSH TABLES WITH READ LOCK; +CREATE TABLE t2 (c1 int); +UNLOCK TABLES; +UNLOCK TABLES; +DROP TABLE t1, t2; +CREATE TABLE t1 (c1 int); +LOCK TABLE t1 WRITE; + FLUSH TABLES WITH READ LOCK; +CREATE TABLE t2 AS SELECT * FROM t1; +ERROR HY000: Table 't2' was not locked with LOCK TABLES +UNLOCK TABLES; +UNLOCK TABLES; +DROP TABLE t1; CREATE DATABASE mysqltest_1; FLUSH TABLES WITH READ LOCK; DROP DATABASE mysqltest_1; diff --git a/mysql-test/t/lock_multi.test b/mysql-test/t/lock_multi.test index 5bebec49b88..37b8fbda376 100644 --- a/mysql-test/t/lock_multi.test +++ b/mysql-test/t/lock_multi.test @@ -192,32 +192,6 @@ disconnect con2; DROP DATABASE mysqltest_1; # -# Bug #17264: MySQL Server freeze -# -connection locker; -create table t1 (f1 int(12) unsigned not null auto_increment, primary key(f1)) engine=innodb; -lock tables t1 write; -connection writer; ---sleep 2 -delimiter //; -send alter table t1 auto_increment=0; alter table t1 auto_increment=0; alter table t1 auto_increment=0; alter table t1 auto_increment=0; alter table t1 auto_increment=0; // -delimiter ;// -connection reader; ---sleep 2 -delimiter //; -send alter table t1 auto_increment=0; alter table t1 auto_increment=0; alter table t1 auto_increment=0; alter table t1 auto_increment=0; alter table t1 auto_increment=0; // -delimiter ;// -connection locker; ---sleep 2 -unlock tables; -connection writer; -reap; -connection reader; -reap; -connection locker; -drop table t1; - -# End of 5.0 tests # Bug#16986 - Deadlock condition with MyISAM tables # connection locker; @@ -246,4 +220,81 @@ connection locker; use test; # connection default; +# +# Test if CREATE TABLE with LOCK TABLE deadlocks. +# +connection writer; +CREATE TABLE t1 (c1 int); +LOCK TABLE t1 WRITE; +# +# This waits until t1 is unlocked. +connection locker; +send FLUSH TABLES WITH READ LOCK; +--sleep 1 +# +# This must not block. +connection writer; +CREATE TABLE t2 (c1 int); +UNLOCK TABLES; +# +# This awakes now. +connection locker; +reap; +UNLOCK TABLES; +# +connection default; +DROP TABLE t1, t2; +# +# Test if CREATE TABLE SELECT with LOCK TABLE deadlocks. +# +connection writer; +CREATE TABLE t1 (c1 int); +LOCK TABLE t1 WRITE; +# +# This waits until t1 is unlocked. +connection locker; +send FLUSH TABLES WITH READ LOCK; +--sleep 1 +# +# This must not block. +connection writer; +--error 1100 +CREATE TABLE t2 AS SELECT * FROM t1; +UNLOCK TABLES; +# +# This awakes now. +connection locker; +reap; +UNLOCK TABLES; +# +connection default; +DROP TABLE t1; + +# +# Bug #17264: MySQL Server freeze +# +connection locker; +create table t1 (f1 int(12) unsigned not null auto_increment, primary key(f1)) engine=innodb; +lock tables t1 write; +connection writer; +--sleep 2 +delimiter //; +send alter table t1 auto_increment=0; alter table t1 auto_increment=0; alter table t1 auto_increment=0; alter table t1 auto_increment=0; alter table t1 auto_increment=0; // +delimiter ;// +connection reader; +--sleep 2 +delimiter //; +send alter table t1 auto_increment=0; alter table t1 auto_increment=0; alter table t1 auto_increment=0; alter table t1 auto_increment=0; alter table t1 auto_increment=0; // +delimiter ;// +connection locker; +--sleep 2 +unlock tables; +connection writer; +reap; +connection reader; +reap; +connection locker; +drop table t1; + +# End of 5.0 tests diff --git a/mysys/thr_lock.c b/mysys/thr_lock.c index 74d6f7431a8..36cb83ae754 100644 --- a/mysys/thr_lock.c +++ b/mysys/thr_lock.c @@ -204,6 +204,8 @@ static void check_locks(THR_LOCK *lock, const char *where, { if ((int) data->type == (int) TL_READ_NO_INSERT) count++; + /* Protect against infinite loop. */ + DBUG_ASSERT(count <= lock->read_no_write_count); } if (count != lock->read_no_write_count) { diff --git a/sql/lock.cc b/sql/lock.cc index e5003325df6..8e75ea42f7d 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -935,7 +935,7 @@ void unlock_table_name(THD *thd, TABLE_LIST *table_list) if (table_list->table) { hash_delete(&open_cache, (byte*) table_list->table); - (void) pthread_cond_broadcast(&COND_refresh); + broadcast_refresh(); } } @@ -1037,9 +1037,9 @@ end: (default 0, which will unlock all tables) NOTES - One must have a lock on LOCK_open when calling this - This function will send a COND_refresh signal to inform other threads - that the name locks are removed + One must have a lock on LOCK_open when calling this. + This function will broadcast refresh signals to inform other threads + that the name locks are removed. RETURN 0 ok @@ -1054,7 +1054,7 @@ void unlock_table_names(THD *thd, TABLE_LIST *table_list, table != last_table; table= table->next_local) unlock_table_name(thd,table); - pthread_cond_broadcast(&COND_refresh); + broadcast_refresh(); DBUG_VOID_RETURN; } @@ -1344,3 +1344,39 @@ bool make_global_read_lock_block_commit(THD *thd) thd->exit_cond(old_message); // this unlocks LOCK_global_read_lock DBUG_RETURN(error); } + + +/* + Broadcast COND_refresh and COND_global_read_lock. + + SYNOPSIS + broadcast_refresh() + void No parameters. + + DESCRIPTION + Due to a bug in a threading library it could happen that a signal + did not reach its target. A condition for this was that the same + condition variable was used with different mutexes in + pthread_cond_wait(). Some time ago we changed LOCK_open to + LOCK_global_read_lock in global read lock handling. So COND_refresh + was used with LOCK_open and LOCK_global_read_lock. + + We did now also change from COND_refresh to COND_global_read_lock + in global read lock handling. But now it is necessary to signal + both conditions at the same time. + + NOTE + When signalling COND_global_read_lock within the global read lock + handling, it is not necessary to also signal COND_refresh. + + RETURN + void +*/ + +void broadcast_refresh(void) +{ + VOID(pthread_cond_broadcast(&COND_refresh)); + VOID(pthread_cond_broadcast(&COND_global_read_lock)); +} + + diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index c0b453b7d69..7cff7d7531c 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -1696,6 +1696,7 @@ void start_waiting_global_read_lock(THD *thd); bool make_global_read_lock_block_commit(THD *thd); bool set_protect_against_global_read_lock(void); void unset_protect_against_global_read_lock(void); +void broadcast_refresh(void); /* Lock based on name */ int lock_and_wait_for_table_name(THD *thd, TABLE_LIST *table_list); diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 5b039f6bcc0..c2ebd140720 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -1104,7 +1104,7 @@ void close_thread_tables(THD *thd, bool lock_in_use, bool skip_derived) if (found_old_table) { /* Tell threads waiting for refresh that something has happened */ - VOID(pthread_cond_broadcast(&COND_refresh)); + broadcast_refresh(); } if (!lock_in_use) VOID(pthread_mutex_unlock(&LOCK_open)); @@ -1674,7 +1674,7 @@ TABLE *unlink_open_table(THD *thd, TABLE *list, TABLE *find) } *prev=0; // Notify any 'refresh' threads - pthread_cond_broadcast(&COND_refresh); + broadcast_refresh(); return start; } @@ -2224,7 +2224,7 @@ static bool reopen_table(TABLE *table) if (table->triggers) table->triggers->set_table(table); - VOID(pthread_cond_broadcast(&COND_refresh)); + broadcast_refresh(); error=0; end: @@ -2325,7 +2325,7 @@ bool reopen_tables(THD *thd,bool get_locks,bool in_refresh) { my_afree((gptr) tables); } - VOID(pthread_cond_broadcast(&COND_refresh)); // Signal to refresh + broadcast_refresh(); *prev=0; DBUG_RETURN(error); } @@ -2361,7 +2361,7 @@ void close_old_data_files(THD *thd, TABLE *table, bool abort_locks, } } if (found) - VOID(pthread_cond_broadcast(&COND_refresh)); // Signal to refresh + broadcast_refresh(); DBUG_VOID_RETURN; } @@ -2514,6 +2514,8 @@ TABLE *drop_locked_tables(THD *thd,const char *db, const char *table_name) } } *prev=0; + if (found) + broadcast_refresh(); if (thd->locked_tables && thd->locked_tables->table_count == 0) { my_free((gptr) thd->locked_tables,MYF(0)); @@ -6194,7 +6196,7 @@ bool remove_table_from_cache(THD *thd, const char *db, const char *table_name, Signal any thread waiting for tables to be freed to reopen their tables */ - (void) pthread_cond_broadcast(&COND_refresh); + broadcast_refresh(); DBUG_PRINT("info", ("Waiting for refresh signal")); if (!(flags & RTFC_CHECK_KILLED_FLAG) || !thd->killed) { diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc index bf035401bea..0d893a6c9be 100644 --- a/sql/sql_handler.cc +++ b/sql/sql_handler.cc @@ -254,7 +254,8 @@ err: DESCRIPTION Though this function takes a list of tables, only the first list entry - will be closed. Broadcasts a COND_refresh condition. + will be closed. + Broadcasts refresh if it closed the table. RETURN FALSE ok @@ -291,7 +292,7 @@ bool mysql_ha_close(THD *thd, TABLE_LIST *tables) if (close_thread_table(thd, table_ptr)) { /* Tell threads waiting for refresh that something has happened */ - VOID(pthread_cond_broadcast(&COND_refresh)); + broadcast_refresh(); } VOID(pthread_mutex_unlock(&LOCK_open)); } @@ -615,7 +616,7 @@ err0: tables are closed (if MYSQL_HA_FLUSH_ALL) is set. If 'tables' is NULL and MYSQL_HA_FLUSH_ALL is not set, all HANDLER tables marked for flush are closed. - Broadcasts a COND_refresh condition, for every table closed. + Broadcasts refresh for every table closed. NOTE Since mysql_ha_flush() is called when the base table has to be closed, @@ -712,7 +713,7 @@ int mysql_ha_flush(THD *thd, TABLE_LIST *tables, uint mode_flags, MYSQL_HA_REOPEN_ON_USAGE mark for reopen. DESCRIPTION - Broadcasts a COND_refresh condition, for every table closed. + Broadcasts refresh if it closed the table. The caller must lock LOCK_open. RETURN @@ -750,7 +751,7 @@ static int mysql_ha_flush_table(THD *thd, TABLE **table_ptr, uint mode_flags) if (close_thread_table(thd, table_ptr)) { /* Tell threads waiting for refresh that something has happened */ - VOID(pthread_cond_broadcast(&COND_refresh)); + broadcast_refresh(); } DBUG_RETURN(0); diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 088bd3e59e5..ce580e8ab3e 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -1421,18 +1421,6 @@ static TABLE *delayed_get_table(THD *thd,TABLE_LIST *table_list) */ if (! (tmp= find_handler(thd, table_list))) { - /* - Avoid that a global read lock steps in while we are creating the - new thread. It would block trying to open the table. Hence, the - DI thread and this thread would wait until after the global - readlock is gone. Since the insert thread needs to wait for a - global read lock anyway, we do it right now. Note that - wait_if_global_read_lock() sets a protection against a new - global read lock when it succeeds. This needs to be released by - start_waiting_global_read_lock(). - */ - if (wait_if_global_read_lock(thd, 0, 1)) - goto err; if (!(tmp=new delayed_insert())) { my_error(ER_OUTOFMEMORY,MYF(0),sizeof(delayed_insert)); @@ -1473,11 +1461,6 @@ static TABLE *delayed_get_table(THD *thd,TABLE_LIST *table_list) pthread_cond_wait(&tmp->cond_client,&tmp->mutex); } pthread_mutex_unlock(&tmp->mutex); - /* - Release the protection against the global read lock and wake - everyone, who might want to set a global read lock. - */ - start_waiting_global_read_lock(thd); thd->proc_info="got old table"; if (tmp->thd.killed) { @@ -1513,11 +1496,6 @@ static TABLE *delayed_get_table(THD *thd,TABLE_LIST *table_list) err1: thd->fatal_error(); - /* - Release the protection against the global read lock and wake - everyone, who might want to set a global read lock. - */ - start_waiting_global_read_lock(thd); err: pthread_mutex_unlock(&LOCK_delayed_create); DBUG_RETURN(0); // Continue with normal insert @@ -2876,7 +2854,7 @@ bool select_create::send_eof() if (!table->s->tmp_table) { if (close_thread_table(thd, &table)) - VOID(pthread_cond_broadcast(&COND_refresh)); + broadcast_refresh(); } thd->extra_lock=0; table=0; @@ -2906,7 +2884,7 @@ void select_create::abort() quick_rm_table(table_type, create_table->db, create_table->table_name); /* Tell threads waiting for refresh that something has happened */ if (version != refresh_version) - VOID(pthread_cond_broadcast(&COND_refresh)); + broadcast_refresh(); } else if (!create_info->table_existed) close_temporary_table(thd, table, 1, 1); diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 232df095816..57c8895fa31 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -2390,17 +2390,37 @@ static void reset_one_shot_variables(THD *thd) } -/**************************************************************************** -** mysql_execute_command -** Execute command saved in thd and current_lex->sql_command -****************************************************************************/ +/* + Execute command saved in thd and current_lex->sql_command + + SYNOPSIS + mysql_execute_command() + thd Thread handle + + IMPLEMENTATION + + Before every operation that can request a write lock for a table + wait if a global read lock exists. However do not wait if this + thread has locked tables already. No new locks can be requested + until the other locks are released. The thread that requests the + global read lock waits for write locked tables to become unlocked. + + Note that wait_if_global_read_lock() sets a protection against a new + global read lock when it succeeds. This needs to be released by + start_waiting_global_read_lock() after the operation. + + RETURN + FALSE OK + TRUE Error +*/ bool mysql_execute_command(THD *thd) { - bool res= FALSE; - int result= 0; - LEX *lex= thd->lex; + bool res= FALSE; + bool need_start_waiting= FALSE; // have protection against global read lock + int result= 0; + LEX *lex= thd->lex; /* first SELECT_LEX (have special meaning for many of non-SELECTcommands) */ SELECT_LEX *select_lex= &lex->select_lex; /* first table of first SELECT_LEX */ @@ -2865,7 +2885,8 @@ mysql_execute_command(THD *thd) TABLE in the same way. That way we avoid that a new table is created during a gobal read lock. */ - if (wait_if_global_read_lock(thd, 0, 1)) + if (!thd->locked_tables && + !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) { res= 1; goto end_with_restore_list; @@ -2901,7 +2922,7 @@ mysql_execute_command(THD *thd) { update_non_unique_table_error(create_table, "CREATE", duplicate); res= 1; - goto end_with_restart_wait; + goto end_with_restore_list; } } /* If we create merge table, we have to test tables in merge, too */ @@ -2917,7 +2938,7 @@ mysql_execute_command(THD *thd) { update_non_unique_table_error(tab, "CREATE", duplicate); res= 1; - goto end_with_restart_wait; + goto end_with_restore_list; } } } @@ -2962,13 +2983,6 @@ mysql_execute_command(THD *thd) send_ok(thd); } -end_with_restart_wait: - /* - Release the protection against the global read lock and wake - everyone, who might want to set a global read lock. - */ - start_waiting_global_read_lock(thd); - /* put tables back for PS rexecuting */ end_with_restore_list: lex->link_first_table_back(create_table, link_to_local); @@ -3089,6 +3103,13 @@ end_with_restore_list: if (end_active_trans(thd)) goto error; + if (!thd->locked_tables && + !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + { + res= 1; + break; + } + thd->enable_slow_log= opt_log_slow_admin_statements; res= mysql_alter_table(thd, select_lex->db, lex->name, &lex->create_info, @@ -3345,6 +3366,14 @@ end_with_restore_list: break; /* Skip first table, which is the table we are inserting in */ select_lex->context.table_list= first_table->next_local; + + if (!thd->locked_tables && + !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + { + res= 1; + break; + } + res= mysql_insert(thd, all_tables, lex->field_list, lex->many_values, lex->update_list, lex->value_list, lex->duplicates, lex->ignore); @@ -3368,6 +3397,14 @@ end_with_restore_list: select_lex->options|= SELECT_NO_UNLOCK; unit->set_limit(select_lex); + + if (! thd->locked_tables && + ! (need_start_waiting= ! wait_if_global_read_lock(thd, 0, 1))) + { + res= 1; + break; + } + if (!(res= open_and_lock_tables(thd, all_tables))) { /* Skip first table, which is the table we are inserting in */ @@ -3435,6 +3472,14 @@ end_with_restore_list: break; DBUG_ASSERT(select_lex->offset_limit == 0); unit->set_limit(select_lex); + + if (!thd->locked_tables && + !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + { + res= 1; + break; + } + res = mysql_delete(thd, all_tables, select_lex->where, &select_lex->order_list, unit->select_limit_cnt, select_lex->options, @@ -3448,6 +3493,13 @@ end_with_restore_list: (TABLE_LIST *)thd->lex->auxilliary_table_list.first; multi_delete *result; + if (!thd->locked_tables && + !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + { + res= 1; + break; + } + if ((res= multi_delete_precheck(thd, all_tables))) break; @@ -5178,11 +5230,22 @@ end: */ if (!(sql_command_flags[lex->sql_command] & CF_HAS_ROW_COUNT)) thd->row_count_func= -1; - DBUG_RETURN(res || thd->net.report_error); + + goto finish; error: - res= 1; // would be better to set res=1 before "goto error" - goto end; + res= TRUE; + +finish: + if (need_start_waiting) + { + /* + Release the protection against the global read lock and wake + everyone, who might want to set a global read lock. + */ + start_waiting_global_read_lock(thd); + } + DBUG_RETURN(res || thd->net.report_error); } diff --git a/sql/sql_table.cc b/sql/sql_table.cc index d5d33373e6c..22fbd790f6b 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -3312,8 +3312,7 @@ bool mysql_create_table_internal(THD *thd, my_error(ER_TABLE_EXISTS_ERROR, MYF(0), alias); goto err; } - if (wait_if_global_read_lock(thd, 0, 1)) - goto err; + VOID(pthread_mutex_lock(&LOCK_open)); if (!internal_tmp_table && !(create_info->options & HA_LEX_CREATE_TMP_TABLE)) { @@ -3389,7 +3388,6 @@ bool mysql_create_table_internal(THD *thd, error= FALSE; unlock_and_end: VOID(pthread_mutex_unlock(&LOCK_open)); - start_waiting_global_read_lock(thd); err: thd->proc_info="After create"; @@ -3621,7 +3619,7 @@ void close_cached_table(THD *thd, TABLE *table) thd->open_tables=unlink_open_table(thd,thd->open_tables,table); /* When lock on LOCK_open is freed other threads can continue */ - pthread_cond_broadcast(&COND_refresh); + broadcast_refresh(); DBUG_VOID_RETURN; } @@ -6133,7 +6131,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, } } VOID(pthread_mutex_unlock(&LOCK_open)); - VOID(pthread_cond_broadcast(&COND_refresh)); + broadcast_refresh(); /* The ALTER TABLE is always in its own transaction. Commit must not be called while LOCK_open is locked. It could call