diff options
| -rw-r--r-- | mysql-test/r/lock_multi.result | 30 | ||||
| -rw-r--r-- | mysql-test/r/trigger_notembedded.result | 14 | ||||
| -rw-r--r-- | mysql-test/t/lock_multi.test | 98 | ||||
| -rw-r--r-- | mysql-test/t/trigger_notembedded.test | 6 | ||||
| -rw-r--r-- | sql/ha_ndbcluster.cc | 4 | ||||
| -rw-r--r-- | sql/ha_ndbcluster_binlog.cc | 12 | ||||
| -rw-r--r-- | sql/mysql_priv.h | 3 | ||||
| -rw-r--r-- | sql/set_var.cc | 2 | ||||
| -rw-r--r-- | sql/sql_base.cc | 69 | ||||
| -rw-r--r-- | sql/sql_parse.cc | 7 | ||||
| -rw-r--r-- | sql/sql_table.cc | 2 |
11 files changed, 206 insertions, 41 deletions
diff --git a/mysql-test/r/lock_multi.result b/mysql-test/r/lock_multi.result index 4a0f70a7b88..9c4f1b17dcc 100644 --- a/mysql-test/r/lock_multi.result +++ b/mysql-test/r/lock_multi.result @@ -113,4 +113,34 @@ handler t1 open; ERROR HY000: Table storage engine for 't1' doesn't have this option --> client 1 drop table t1; +drop table if exists t1; +create table t1 (i int); +connection: default +lock tables t1 write; +connection: flush +flush tables with read lock;; +connection: default +alter table t1 add column j int; +connection: insert +insert into t1 values (1,2);; +connection: default +unlock tables; +connection: flush +select * from t1; +i j +unlock tables; +select * from t1; +i j +1 2 +drop table t1; +drop table if exists t1; +create table t1 (i int); +connection: default +lock tables t1 write; +connection: flush +flush tables with read lock;; +connection: default +flush tables; +unlock tables; +drop table t1; End of 5.1 tests diff --git a/mysql-test/r/trigger_notembedded.result b/mysql-test/r/trigger_notembedded.result index d56f83993a6..87e8f68da38 100644 --- a/mysql-test/r/trigger_notembedded.result +++ b/mysql-test/r/trigger_notembedded.result @@ -448,4 +448,18 @@ DROP TABLE t1; DROP DATABASE mysqltest_db1; USE test; End of 5.0 tests. +drop table if exists t1; +create table t1 (i int); +connection: default +lock tables t1 write; +connection: flush +flush tables with read lock;; +connection: default +create trigger t1_bi before insert on t1 for each row begin end; +unlock tables; +connection: flush +unlock tables; +select * from t1; +i +drop table t1; End of 5.1 tests. diff --git a/mysql-test/t/lock_multi.test b/mysql-test/t/lock_multi.test index b2266c9bff1..0d36b79df78 100644 --- a/mysql-test/t/lock_multi.test +++ b/mysql-test/t/lock_multi.test @@ -150,7 +150,7 @@ send SELECT user.Select_priv FROM user, db WHERE user.user = db.user LIMIT 1; connection locker; let $wait_condition= select count(*) = 1 from information_schema.processlist - where state = "Locked" and info = + where state = "Waiting for table" and info = "SELECT user.Select_priv FROM user, db WHERE user.user = db.user LIMIT 1"; --source include/wait_condition.inc # Make test case independent from earlier grants. @@ -343,4 +343,100 @@ handler t1 open; connection default; drop table t1; +# +# Bug#32395 Alter table under a impending global read lock causes a server crash +# + +# +# Test ALTER TABLE under LOCK TABLES and FLUSH TABLES WITH READ LOCK +# + +--disable_warnings +drop table if exists t1; +--enable_warnings +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 +alter table t1 add column j int; +connect (insert,localhost,root,,test,,); +connection insert; +--echo connection: insert +let $wait_condition= + select count(*) = 1 from information_schema.processlist + where state = "Flushing tables"; +--source include/wait_condition.inc +--send insert into t1 values (1,2); +--echo connection: default +connection default; +let $wait_condition= + select count(*) = 1 from information_schema.processlist + where state = "Waiting for release of readlock"; +--source include/wait_condition.inc +unlock tables; +connection flush; +--echo connection: flush +--reap +let $wait_condition= + select count(*) = 1 from information_schema.processlist + where state = "Waiting for release of readlock"; +--source include/wait_condition.inc +select * from t1; +unlock tables; +connection insert; +--reap +connection default; +select * from t1; +drop table t1; +disconnect flush; +disconnect insert; + +# +# Test that FLUSH TABLES under LOCK TABLES protects write locked tables +# from a impending FLUSH TABLES WITH READ LOCK +# + +--disable_warnings +drop table if exists t1; +--enable_warnings +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 +flush tables; +let $wait_condition= + select count(*) = 1 from information_schema.processlist + where state = "Flushing tables"; +--source include/wait_condition.inc +unlock tables; +let $wait_condition= + select count(*) = 0 from information_schema.processlist + where state = "Flushing tables"; +--source include/wait_condition.inc +connection flush; +--reap +connection default; +disconnect flush; +drop table t1; + --echo End of 5.1 tests diff --git a/mysql-test/t/trigger_notembedded.test b/mysql-test/t/trigger_notembedded.test index 748ae6e1c27..5d2ab84adaf 100644 --- a/mysql-test/t/trigger_notembedded.test +++ b/mysql-test/t/trigger_notembedded.test @@ -880,8 +880,9 @@ USE test; # Bug#23713 LOCK TABLES + CREATE TRIGGER + FLUSH TABLES WITH READ LOCK = deadlock # -# Test temporarily disable due to Bug#32395 ---disable_parsing +--disable_warnings +drop table if exists t1; +--enable_warnings create table t1 (i int); connect (flush,localhost,root,,test,,); connection default; @@ -906,6 +907,5 @@ connection default; select * from t1; drop table t1; disconnect flush; ---enable_parsing --echo End of 5.1 tests. diff --git a/sql/ha_ndbcluster.cc b/sql/ha_ndbcluster.cc index bf2b19bfc9c..2555bb23825 100644 --- a/sql/ha_ndbcluster.cc +++ b/sql/ha_ndbcluster.cc @@ -577,7 +577,7 @@ int ha_ndbcluster::ndb_err(NdbTransaction *trans) bzero((char*) &table_list,sizeof(table_list)); table_list.db= m_dbname; table_list.alias= table_list.table_name= m_tabname; - close_cached_tables(thd, 0, &table_list); + close_cached_tables(thd, &table_list, FALSE, FALSE, FALSE); break; } default: @@ -8064,7 +8064,7 @@ int handle_trailing_share(NDB_SHARE *share) table_list.db= share->db; table_list.alias= table_list.table_name= share->table_name; safe_mutex_assert_owner(&LOCK_open); - close_cached_tables(thd, 0, &table_list, TRUE); + close_cached_tables(thd, &table_list, TRUE, FALSE, FALSE); pthread_mutex_lock(&ndbcluster_mutex); /* ndb_share reference temporary free */ diff --git a/sql/ha_ndbcluster_binlog.cc b/sql/ha_ndbcluster_binlog.cc index be75eff2575..a9d1615a1b4 100644 --- a/sql/ha_ndbcluster_binlog.cc +++ b/sql/ha_ndbcluster_binlog.cc @@ -883,7 +883,7 @@ int ndbcluster_setup_binlog_table_shares(THD *thd) { if (ndb_extra_logging) sql_print_information("NDB Binlog: ndb tables writable"); - close_cached_tables((THD*) 0, 0, (TABLE_LIST*) 0, TRUE); + close_cached_tables(NULL, NULL, TRUE, FALSE, FALSE); } pthread_mutex_unlock(&LOCK_open); /* Signal injector thread that all is setup */ @@ -1683,7 +1683,7 @@ ndb_handle_schema_change(THD *thd, Ndb *ndb, NdbEventOperation *pOp, bzero((char*) &table_list,sizeof(table_list)); table_list.db= (char *)dbname; table_list.alias= table_list.table_name= (char *)tabname; - close_cached_tables(thd, 0, &table_list, TRUE); + close_cached_tables(thd, &table_list, TRUE, FALSE, FALSE); if ((error= ndbcluster_binlog_open_table(thd, share, table_share, table, 1))) @@ -1789,7 +1789,7 @@ ndb_handle_schema_change(THD *thd, Ndb *ndb, NdbEventOperation *pOp, bzero((char*) &table_list,sizeof(table_list)); table_list.db= (char *)dbname; table_list.alias= table_list.table_name= (char *)tabname; - close_cached_tables(thd, 0, &table_list); + close_cached_tables(thd, &table_list, FALSE, FALSE, FALSE); /* ndb_share reference create free */ DBUG_PRINT("NDB_SHARE", ("%s create free use_count: %u", share->key, share->use_count)); @@ -1908,7 +1908,7 @@ ndb_binlog_thread_handle_schema_event(THD *thd, Ndb *ndb, bzero((char*) &table_list,sizeof(table_list)); table_list.db= schema->db; table_list.alias= table_list.table_name= schema->name; - close_cached_tables(thd, 0, &table_list, FALSE); + close_cached_tables(thd, &table_list, FALSE, FALSE, FALSE); } /* ndb_share reference temporary free */ if (share) @@ -2032,7 +2032,7 @@ ndb_binlog_thread_handle_schema_event(THD *thd, Ndb *ndb, pthread_mutex_unlock(&ndb_schema_share_mutex); /* end protect ndb_schema_share */ - close_cached_tables((THD*) 0, 0, (TABLE_LIST*) 0, FALSE); + close_cached_tables(NULL, NULL, FALSE, FALSE, FALSE); // fall through case NDBEVENT::TE_ALTER: ndb_handle_schema_change(thd, ndb, pOp, tmp_share); @@ -2189,7 +2189,7 @@ ndb_binlog_thread_handle_schema_event_post_epoch(THD *thd, bzero((char*) &table_list,sizeof(table_list)); table_list.db= schema->db; table_list.alias= table_list.table_name= schema->name; - close_cached_tables(thd, 0, &table_list, FALSE); + close_cached_tables(thd, &table_list, FALSE, FALSE, FALSE); } if (schema_type != SOT_ALTER_TABLE) break; diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index cafb7487e35..46e08191480 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -1595,7 +1595,8 @@ TABLE *open_performance_schema_table(THD *thd, TABLE_LIST *one_table, Open_tables_state *backup); void close_performance_schema_table(THD *thd, Open_tables_state *backup); -bool close_cached_tables(THD *thd, bool wait_for_refresh, TABLE_LIST *tables, bool have_lock = FALSE); +bool close_cached_tables(THD *thd, TABLE_LIST *tables, bool have_lock, + bool wait_for_refresh, bool wait_for_placeholders); bool close_cached_connection_tables(THD *thd, bool wait_for_refresh, LEX_STRING *connect_string, bool have_lock = FALSE); diff --git a/sql/set_var.cc b/sql/set_var.cc index d408b2de64e..e397eccbe8b 100644 --- a/sql/set_var.cc +++ b/sql/set_var.cc @@ -3685,7 +3685,7 @@ bool sys_var_opt_readonly::update(THD *thd, set_var *var) can cause to wait on a read lock, it's required for the client application to unlock everything, and acceptable for the server to wait on all locks. */ - if (result= close_cached_tables(thd, true, NULL, false)) + if (result= close_cached_tables(thd, NULL, FALSE, TRUE, TRUE)) goto end_with_read_lock; if (result= make_global_read_lock_block_commit(thd)) diff --git a/sql/sql_base.cc b/sql/sql_base.cc index ba8b7fc1330..fd8e107b85a 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -130,7 +130,7 @@ void table_cache_free(void) DBUG_ENTER("table_cache_free"); if (table_def_inited) { - close_cached_tables((THD*) 0,0,(TABLE_LIST*) 0); + close_cached_tables(NULL, NULL, FALSE, FALSE, FALSE); if (!open_cache.records) // Safety first hash_free(&open_cache); } @@ -885,16 +885,24 @@ void free_io_cache(TABLE *table) /* Close all tables which aren't in use by any thread - THD can be NULL, but then if_wait_for_refresh must be FALSE - and tables must be NULL. + @param thd Thread context + @param tables List of tables to remove from the cache + @param have_lock If LOCK_open is locked + @param wait_for_refresh Wait for a impending flush + @param wait_for_placeholders Wait for tables being reopened so that the GRL + won't proceed while write-locked tables are being reopened by other + threads. + + @remark THD can be NULL, but then wait_for_refresh must be FALSE + and tables must be NULL. */ -bool close_cached_tables(THD *thd, bool if_wait_for_refresh, - TABLE_LIST *tables, bool have_lock) +bool close_cached_tables(THD *thd, TABLE_LIST *tables, bool have_lock, + bool wait_for_refresh, bool wait_for_placeholders) { bool result=0; DBUG_ENTER("close_cached_tables"); - DBUG_ASSERT(thd || (!if_wait_for_refresh && !tables)); + DBUG_ASSERT(thd || (!wait_for_refresh && !tables)); if (!have_lock) VOID(pthread_mutex_lock(&LOCK_open)); @@ -918,7 +926,7 @@ bool close_cached_tables(THD *thd, bool if_wait_for_refresh, } DBUG_PRINT("tcache", ("incremented global refresh_version to: %lu", refresh_version)); - if (if_wait_for_refresh) + if (wait_for_refresh) { /* Other threads could wait in a loop in open_and_lock_tables(), @@ -975,13 +983,13 @@ bool close_cached_tables(THD *thd, bool if_wait_for_refresh, found=1; } if (!found) - if_wait_for_refresh=0; // Nothing to wait for + wait_for_refresh=0; // Nothing to wait for } #ifndef EMBEDDED_LIBRARY if (!tables) kill_delayed_threads(); #endif - if (if_wait_for_refresh) + if (wait_for_refresh) { /* If there is any table that has a lower refresh_version, wait until @@ -1004,6 +1012,9 @@ bool close_cached_tables(THD *thd, bool if_wait_for_refresh, for (uint idx=0 ; idx < open_cache.records ; idx++) { TABLE *table=(TABLE*) hash_element(&open_cache,idx); + /* Avoid a self-deadlock. */ + if (table->in_use == thd) + continue; /* Note that we wait here only for tables which are actually open, and not for placeholders with TABLE::open_placeholder set. Waiting for @@ -1018,7 +1029,8 @@ bool close_cached_tables(THD *thd, bool if_wait_for_refresh, are employed by CREATE TABLE as in this case table simply does not exist yet. */ - if (table->needs_reopen_or_name_lock() && table->db_stat) + if (table->needs_reopen_or_name_lock() && (table->db_stat || + (table->open_placeholder && wait_for_placeholders))) { found=1; DBUG_PRINT("signal", ("Waiting for COND_refresh")); @@ -1037,11 +1049,18 @@ bool close_cached_tables(THD *thd, bool if_wait_for_refresh, thd->in_lock_tables=0; /* Set version for table */ for (TABLE *table=thd->open_tables; table ; table= table->next) - table->s->version= refresh_version; + { + /* + Preserve the version (0) of write locked tables so that a impending + global read lock won't sneak in. + */ + if (table->reginfo.lock_type < TL_WRITE_ALLOW_WRITE) + table->s->version= refresh_version; + } } if (!have_lock) VOID(pthread_mutex_unlock(&LOCK_open)); - if (if_wait_for_refresh) + if (wait_for_refresh) { pthread_mutex_lock(&thd->mysys_var->mutex); thd->mysys_var->current_mutex= 0; @@ -1068,10 +1087,10 @@ bool close_cached_connection_tables(THD *thd, bool if_wait_for_refresh, DBUG_ASSERT(thd); bzero(&tmp, sizeof(TABLE_LIST)); - + if (!have_lock) VOID(pthread_mutex_lock(&LOCK_open)); - + for (idx= 0; idx < table_def_cache.records; idx++) { TABLE_SHARE *share= (TABLE_SHARE *) hash_element(&table_def_cache, idx); @@ -1100,11 +1119,11 @@ bool close_cached_connection_tables(THD *thd, bool if_wait_for_refresh, } if (tables) - result= close_cached_tables(thd, FALSE, tables, TRUE); - + result= close_cached_tables(thd, tables, TRUE, FALSE, FALSE); + if (!have_lock) VOID(pthread_mutex_unlock(&LOCK_open)); - + if (if_wait_for_refresh) { pthread_mutex_lock(&thd->mysys_var->mutex); @@ -2204,7 +2223,7 @@ void wait_for_condition(THD *thd, pthread_mutex_t *mutex, pthread_cond_t *cond) current thread. @param thd current thread context - @param tables able list containing one table to open. + @param tables table list containing one table to open. @return FALSE on success, TRUE otherwise. */ @@ -3272,8 +3291,8 @@ static bool reattach_merge(THD *thd, TABLE **err_tables_p) @param thd Thread context @param get_locks Should we get locks after reopening tables ? - @param in_refresh Are we in FLUSH TABLES ? TODO: It seems that - we can remove this parameter. + @param mark_share_as_old Mark share as old to protect from a impending + global read lock. @note Since this function can't properly handle prelocking and create placeholders it should be used in very special @@ -3287,13 +3306,17 @@ static bool reattach_merge(THD *thd, TABLE **err_tables_p) @return FALSE in case of success, TRUE - otherwise. */ -bool reopen_tables(THD *thd,bool get_locks,bool in_refresh) +bool reopen_tables(THD *thd, bool get_locks, bool mark_share_as_old) { TABLE *table,*next,**prev; TABLE **tables,**tables_ptr; // For locks TABLE *err_tables= NULL; bool error=0, not_used; bool merge_table_found= FALSE; + const uint flags= MYSQL_LOCK_NOTIFY_IF_NEED_REOPEN | + MYSQL_LOCK_IGNORE_GLOBAL_READ_LOCK | + MYSQL_LOCK_IGNORE_FLUSH; + DBUG_ENTER("reopen_tables"); if (!thd->open_tables) @@ -3354,7 +3377,7 @@ bool reopen_tables(THD *thd,bool get_locks,bool in_refresh) /* Do not handle locks of MERGE children. */ if (get_locks && !db_stat && !table->parent) *tables_ptr++= table; // need new lock on this - if (in_refresh) + if (mark_share_as_old) { table->s->version=0; table->open_placeholder= 0; @@ -3387,7 +3410,7 @@ bool reopen_tables(THD *thd,bool get_locks,bool in_refresh) */ thd->some_tables_deleted=0; if ((lock= mysql_lock_tables(thd, tables, (uint) (tables_ptr - tables), - MYSQL_LOCK_NOTIFY_IF_NEED_REOPEN, ¬_used))) + flags, ¬_used))) { thd->locked_tables=mysql_lock_merge(thd->locked_tables,lock); } diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 6e84551716d..de8698d7bca 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -6530,8 +6530,8 @@ bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables, tmp_write_to_binlog= 0; if (lock_global_read_lock(thd)) return 1; // Killed - result=close_cached_tables(thd,(options & REFRESH_FAST) ? 0 : 1, - tables); + result= close_cached_tables(thd, tables, FALSE, (options & REFRESH_FAST) ? + FALSE : TRUE, TRUE); if (make_global_read_lock_block_commit(thd)) // Killed { /* Don't leave things in a half-locked state */ @@ -6540,7 +6540,8 @@ bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables, } } else - result=close_cached_tables(thd,(options & REFRESH_FAST) ? 0 : 1, tables); + result= close_cached_tables(thd, tables, FALSE, (options & REFRESH_FAST) ? + FALSE : TRUE, FALSE); my_dbopt_cleanup(); } if (options & REFRESH_HOSTS) diff --git a/sql/sql_table.cc b/sql/sql_table.cc index c618d170fb7..e12edebb95d 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -6665,7 +6665,7 @@ view_err: if (thd->locked_tables && new_name == table_name && new_db == db) { thd->in_lock_tables= 1; - error= reopen_tables(thd, 1, 0); + error= reopen_tables(thd, 1, 1); thd->in_lock_tables= 0; if (error) goto err_with_placeholders; |