From fab67c98ae75e0c9c1a42acb8f4ee3e0c46fb4e5 Mon Sep 17 00:00:00 2001 From: MySQL Build Team Date: Wed, 24 Jun 2009 19:17:12 +0200 Subject: [PATCH] Backport into build-200906240007-5.1.34sp1 > ------------------------------------------------------------ > revno: 2852.2.3 > revision-id: davi.arnaut@sun.com-20090403194600-60ufn0tz1gx1kl0l > parent: gni@mysql.com-20090403184200-vnjtpsv4an79w8bu > parent: davi.arnaut@sun.com-20090403191154-0ho2nai3chjsmpof > committer: Davi Arnaut > branch nick: 43230-5.1 > timestamp: Fri 2009-04-03 16:46:00 -0300 > message: > Merge Bug#43230 into mysql-5.1-bugteam > ------------------------------------------------------------ > revno: 1810.3855.16 > revision-id: davi.arnaut@sun.com-20090403191154-0ho2nai3chjsmpof > parent: chad@mysql.com-20090402152928-3ld60a56h86njcpg > committer: Davi Arnaut > branch nick: 43230-5.0 > timestamp: Fri 2009-04-03 16:11:54 -0300 > message: > Bug#43230: SELECT ... FOR UPDATE can hang with FLUSH TABLES WITH READ LOCK indefinitely > > The problem is that a SELECT .. FOR UPDATE statement might open > a table and later wait for a impeding global read lock without > noticing whether it is holding a table that is being waited upon > the the flush phase of the process that took the global read > lock. > > The same problem also affected the following statements: > > LOCK TABLES .. WRITE > UPDATE .. SET (update and multi-table update) > TRUNCATE TABLE .. > LOAD DATA .. > > The solution is to make the above statements wait for a impending > global read lock before opening the tables. If there is no > impending global read lock, the statement raises a temporary > protection against global read locks and progresses smoothly > towards completion. > > Important notice: the patch does not try to address all possible > cases, only those which are common and can be fixed unintrusively > enough for 5.0. --- mysql-test/r/lock_multi.result | 55 ++++++++++++++ mysql-test/t/lock_multi.test | 128 +++++++++++++++++++++++++++++++++ sql/sql_lex.cc | 1 + sql/sql_lex.h | 16 +++++ sql/sql_parse.cc | 33 ++++++++- sql/sql_yacc.yy | 7 +- 6 files changed, 236 insertions(+), 4 deletions(-) diff --git a/mysql-test/r/lock_multi.result b/mysql-test/r/lock_multi.result index 35bc7376296..d8768e802ea 100644 --- a/mysql-test/r/lock_multi.result +++ b/mysql-test/r/lock_multi.result @@ -96,6 +96,61 @@ alter table t1 auto_increment=0; alter table t1 auto_increment=0; unlock tables; drop table t1; +create table t1 (a int); +create table t2 like t1; +# con1 +lock tables t1 write; +# con2 +flush tables with read lock; +# con5 +# global read lock is taken +# con3 +select * from t2 for update; +# waiting for release of read lock +# con4 +# would hang and later cause a deadlock +flush tables t2; +# clean up +unlock tables; +unlock tables; +a +drop table t1,t2; +# +# Lightweight version: +# Ensure that the wait for a GRL is done before opening tables. +# +create table t1 (a int); +create table t2 like t1; +# +# UPDATE +# +# default +flush tables with read lock; +# con1 +update t2 set a = 1; +# default +# statement is waiting for release of read lock +# con2 +flush table t2; +# default +unlock tables; +# con1 +# +# LOCK TABLES .. WRITE +# +# default +flush tables with read lock; +# con1 +lock tables t2 write; +# default +# statement is waiting for release of read lock +# con2 +flush table t2; +# default +unlock tables; +# con1 +unlock tables; +drop table t1,t2; End of 5.0 tests create table t1 (i int); lock table t1 read; diff --git a/mysql-test/t/lock_multi.test b/mysql-test/t/lock_multi.test index e93153f005c..47b5aa0292b 100644 --- a/mysql-test/t/lock_multi.test +++ b/mysql-test/t/lock_multi.test @@ -317,6 +317,134 @@ reap; connection locker; drop table t1; +# +# Bug#43230: SELECT ... FOR UPDATE can hang with FLUSH TABLES WITH READ LOCK indefinitely +# + +connect (con1,localhost,root,,); +connect (con2,localhost,root,,); +connect (con3,localhost,root,,); +connect (con4,localhost,root,,); +connect (con5,localhost,root,,); + +create table t1 (a int); +create table t2 like t1; + +connection con1; +--echo # con1 +lock tables t1 write; +connection con2; +--echo # con2 +send flush tables with read lock; +connection con5; +--echo # con5 +let $show_statement= SHOW PROCESSLIST; +let $field= State; +let $condition= = 'Flushing tables'; +--source include/wait_show_condition.inc +--echo # global read lock is taken +connection con3; +--echo # con3 +send select * from t2 for update; +connection con5; +let $show_statement= SHOW PROCESSLIST; +let $field= State; +let $condition= = 'Waiting for release of readlock'; +--source include/wait_show_condition.inc +--echo # waiting for release of read lock +connection con4; +--echo # con4 +--echo # would hang and later cause a deadlock +flush tables t2; +connection con1; +--echo # clean up +unlock tables; +connection con2; +--reap +unlock tables; +connection con3; +--reap +connection default; +disconnect con5; +disconnect con4; +disconnect con3; +disconnect con2; +disconnect con1; + +drop table t1,t2; + +--echo # +--echo # Lightweight version: +--echo # Ensure that the wait for a GRL is done before opening tables. +--echo # + +connect (con1,localhost,root,,); +connect (con2,localhost,root,,); + +create table t1 (a int); +create table t2 like t1; + +--echo # +--echo # UPDATE +--echo # + +connection default; +--echo # default +flush tables with read lock; +connection con1; +--echo # con1 +send update t2 set a = 1; +connection default; +--echo # default +let $show_statement= SHOW PROCESSLIST; +let $field= State; +let $condition= = 'Waiting for release of readlock'; +--source include/wait_show_condition.inc +--echo # statement is waiting for release of read lock +connection con2; +--echo # con2 +flush table t2; +connection default; +--echo # default +unlock tables; +connection con1; +--echo # con1 +--reap + +--echo # +--echo # LOCK TABLES .. WRITE +--echo # + +connection default; +--echo # default +flush tables with read lock; +connection con1; +--echo # con1 +send lock tables t2 write; +connection default; +--echo # default +let $show_statement= SHOW PROCESSLIST; +let $field= State; +let $condition= = 'Waiting for release of readlock'; +--source include/wait_show_condition.inc +--echo # statement is waiting for release of read lock +connection con2; +--echo # con2 +flush table t2; +connection default; +--echo # default +unlock tables; +connection con1; +--echo # con1 +--reap +unlock tables; + +connection default; +disconnect con2; +disconnect con1; + +drop table t1,t2; + --echo End of 5.0 tests diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index e6eb9a8a44e..69b8c486d5f 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -348,6 +348,7 @@ void lex_start(THD *thd) lex->nest_level=0 ; lex->allow_sum_func= 0; lex->in_sum_func= NULL; + lex->protect_against_global_read_lock= FALSE; /* ok, there must be a better solution for this, long-term I tried "bzero" in the sql_yacc.yy code, but that for diff --git a/sql/sql_lex.h b/sql/sql_lex.h index f34a1c7c36f..172940cf9d7 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -1745,6 +1745,22 @@ typedef struct st_lex : public Query_tables_list bool escape_used; bool is_lex_started; /* If lex_start() did run. For debugging. */ + /* + Special case for SELECT .. FOR UPDATE and LOCK TABLES .. WRITE. + + Protect from a impending GRL as otherwise the thread might deadlock + if it starts waiting for the GRL in mysql_lock_tables. + + The protection is needed because there is a race between setting + the global read lock and waiting for all open tables to be closed. + The problem is a circular wait where a thread holding "old" open + tables will wait for the global read lock to be released while the + thread holding the global read lock will wait for all "old" open + tables to be closed -- the flush part of flush tables with read + lock. + */ + bool protect_against_global_read_lock; + st_lex(); virtual ~st_lex() diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 2974dff9ea9..6dbe4a4fd8d 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -2200,8 +2200,15 @@ mysql_execute_command(THD *thd) res= check_access(thd, lex->exchange ? SELECT_ACL | FILE_ACL : SELECT_ACL, any_db, 0, 0, 0, 0); - if (!res) - res= execute_sqlcom_select(thd, all_tables); + + if (res) + break; + + if (!thd->locked_tables && lex->protect_against_global_read_lock && + !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + break; + + res= execute_sqlcom_select(thd, all_tables); break; case SQLCOM_PREPARE: { @@ -3006,6 +3013,9 @@ end_with_restore_list: DBUG_ASSERT(first_table == all_tables && first_table != 0); if (update_precheck(thd, all_tables)) break; + if (!thd->locked_tables && + !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + goto error; DBUG_ASSERT(select_lex->offset_limit == 0); unit->set_limit(select_lex); res= (up_result= mysql_update(thd, all_tables, @@ -3032,6 +3042,15 @@ end_with_restore_list: else res= 0; + /* + Protection might have already been risen if its a fall through + from the SQLCOM_UPDATE case above. + */ + if (!thd->locked_tables && + lex->sql_command == SQLCOM_UPDATE_MULTI && + !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + goto error; + res= mysql_multi_update_prepare(thd); #ifdef HAVE_REPLICATION @@ -3229,7 +3248,8 @@ end_with_restore_list: ER(ER_LOCK_OR_ACTIVE_TRANSACTION), MYF(0)); goto error; } - + if (!(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + goto error; res= mysql_truncate(thd, first_table, 0); break; case SQLCOM_DELETE: @@ -3402,6 +3422,10 @@ end_with_restore_list: if (check_one_table_access(thd, privilege, all_tables)) goto error; + if (!thd->locked_tables && + !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + goto error; + res= mysql_load(thd, lex->exchange, first_table, lex->field_list, lex->update_list, lex->value_list, lex->duplicates, lex->ignore, (bool) lex->local_file); @@ -3472,6 +3496,9 @@ end_with_restore_list: if (check_table_access(thd, LOCK_TABLES_ACL | SELECT_ACL, all_tables, UINT_MAX, FALSE)) goto error; + if (lex->protect_against_global_read_lock && + !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + goto error; thd->in_lock_tables=1; thd->options|= OPTION_TABLE_LOCK; diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index e56ff7c6ad7..7752ccce73d 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -6538,6 +6538,7 @@ select_lock_type: lex->current_select->set_lock_for_tables(TL_WRITE); lex->current_select->lock_option= TL_WRITE; lex->safe_to_cache_query=0; + lex->protect_against_global_read_lock= TRUE; } | LOCK_SYM IN_SYM SHARE_SYM MODE_SYM { @@ -12182,8 +12183,12 @@ table_lock_list: table_lock: table_ident opt_table_alias lock_option { - if (!Select->add_table_to_list(YYTHD, $1, $2, 0, (thr_lock_type) $3)) + thr_lock_type lock_type= (thr_lock_type) $3; + if (!Select->add_table_to_list(YYTHD, $1, $2, 0, lock_type)) MYSQL_YYABORT; + /* If table is to be write locked, protect from a impending GRL. */ + if (lock_type >= TL_WRITE_ALLOW_WRITE) + Lex->protect_against_global_read_lock= TRUE; } ;