# At a local mysql-5.0-bugteam repository of davi
2726 Davi Arnaut 2009-04-03
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)
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 widespread and can be fixed unintrusively
enough for 5.0.
@ mysql-test/r/lock_multi.result
Add test case result for Bug#43230
@ mysql-test/t/lock_multi.test
Add test case for Bug#43230
@ sql/sql_delete.cc
Wait for the global read lock before opening tables. Move from
the caller because this function is also used by truncate table.
@ sql/sql_lex.cc
Initialize flag.
@ sql/sql_lex.h
Add a flag to the lexer.
@ sql/sql_parse.cc
Wait for the global read lock is a write lock is going to be
taken. The wait is done before opening tables.
Wait for GRL in DELETE is now done within mysql_delete.
@ sql/sql_yacc.yy
tect against the GRL if its a SELECT .. FOR UPDATE or LOCK TABLES
.. WRITE statement.
modified:
mysql-test/r/lock_multi.result
mysql-test/t/lock_multi.test
sql/sql_delete.cc
sql/sql_lex.cc
sql/sql_lex.h
sql/sql_parse.cc
sql/sql_yacc.yy
=== modified file 'mysql-test/r/lock_multi.result'
--- a/mysql-test/r/lock_multi.result 2009-03-23 20:19:41 +0000
+++ b/mysql-test/r/lock_multi.result 2009-04-03 17:42:18 +0000
@@ -133,3 +133,58 @@ ALTER TABLE t1 ADD COLUMN a INT;
# 2.2.1. normal mode
# 2.2.2. PS mode
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;
=== modified file 'mysql-test/t/lock_multi.test'
--- a/mysql-test/t/lock_multi.test 2009-03-23 20:19:41 +0000
+++ b/mysql-test/t/lock_multi.test 2009-04-03 17:42:18 +0000
@@ -683,6 +683,134 @@ DROP TABLE t1;
--disconnect locker
--disconnect writer
+#
+# 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;
+
# End of 5.0 tests
# Wait till all disconnects are completed
=== modified file 'sql/sql_delete.cc'
--- a/sql/sql_delete.cc 2008-07-15 14:13:21 +0000
+++ b/sql/sql_delete.cc 2009-04-03 17:42:18 +0000
@@ -40,27 +40,32 @@ bool mysql_delete(THD *thd, TABLE_LIST *
uint usable_index= MAX_KEY;
SELECT_LEX *select_lex= &thd->lex->select_lex;
THD::killed_state killed_status= THD::NOT_KILLED;
+ bool need_start_waiting= FALSE;
DBUG_ENTER("mysql_delete");
+ if (!thd->locked_tables &&
+ !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1)))
+ goto error;
+
if (open_and_lock_tables(thd, table_list))
- DBUG_RETURN(TRUE);
+ goto error;
if (!(table= table_list->table))
{
my_error(ER_VIEW_DELETE_MERGE_VIEW, MYF(0),
table_list->view_db.str, table_list->view_name.str);
- DBUG_RETURN(TRUE);
+ goto error;
}
error= table->file->info(HA_STATUS_VARIABLE | HA_STATUS_NO_LOCK);
if (error)
{
table->file->print_error(error, MYF(0));
- DBUG_RETURN(error);
+ goto error;
}
thd->proc_info="init";
table->map=1;
if (mysql_prepare_delete(thd, table_list, &conds))
- DBUG_RETURN(TRUE);
+ goto error;
/* check ORDER BY even if it can be ignored */
if (order && order->elements)
@@ -79,7 +84,7 @@ bool mysql_delete(THD *thd, TABLE_LIST *
{
delete select;
free_underlaid_joins(thd, &thd->lex->select_lex);
- DBUG_RETURN(TRUE);
+ goto error;
}
}
@@ -89,7 +94,7 @@ bool mysql_delete(THD *thd, TABLE_LIST *
{
my_message(ER_UPDATE_WITHOUT_KEY_IN_SAFE_MODE,
ER(ER_UPDATE_WITHOUT_KEY_IN_SAFE_MODE), MYF(0));
- DBUG_RETURN(TRUE);
+ goto error;
}
select_lex->no_error= thd->lex->ignore;
@@ -138,13 +143,15 @@ bool mysql_delete(THD *thd, TABLE_LIST *
table->quick_keys.clear_all(); // Can't use 'only index'
select=make_select(table, 0, 0, conds, 0, &error);
if (error)
- DBUG_RETURN(TRUE);
+ goto error;
if ((select && select->check_quick(thd, safe_update, limit)) || !limit)
{
delete select;
free_underlaid_joins(thd, select_lex);
thd->row_count_func= 0;
send_ok(thd,0L);
+ if (need_start_waiting)
+ start_waiting_global_read_lock(thd);
/*
We don't need to call reset_auto_increment in this case, because
@@ -165,7 +172,7 @@ bool mysql_delete(THD *thd, TABLE_LIST *
free_underlaid_joins(thd, select_lex);
my_message(ER_UPDATE_WITHOUT_KEY_IN_SAFE_MODE,
ER(ER_UPDATE_WITHOUT_KEY_IN_SAFE_MODE), MYF(0));
- DBUG_RETURN(TRUE);
+ goto error;
}
}
if (options & OPTION_QUICK)
@@ -194,7 +201,7 @@ bool mysql_delete(THD *thd, TABLE_LIST *
{
delete select;
free_underlaid_joins(thd, &thd->lex->select_lex);
- DBUG_RETURN(TRUE);
+ goto error;
}
/*
Filesort has already found and selected the rows we want to delete,
@@ -211,7 +218,7 @@ bool mysql_delete(THD *thd, TABLE_LIST *
{
delete select;
free_underlaid_joins(thd, select_lex);
- DBUG_RETURN(TRUE);
+ goto error;
}
if (usable_index==MAX_KEY)
init_read_record(&info, thd, table, select, 1, 1, FALSE);
@@ -358,7 +365,13 @@ cleanup:
send_ok(thd,deleted);
DBUG_PRINT("info",("%ld records deleted",(long) deleted));
}
+ if (need_start_waiting)
+ start_waiting_global_read_lock(thd);
DBUG_RETURN(error >= 0 || thd->net.report_error);
+error:
+ if (need_start_waiting)
+ start_waiting_global_read_lock(thd);
+ DBUG_RETURN(TRUE);
}
=== modified file 'sql/sql_lex.cc'
--- a/sql/sql_lex.cc 2009-02-10 22:47:54 +0000
+++ b/sql/sql_lex.cc 2009-04-03 17:42:18 +0000
@@ -204,6 +204,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;
DBUG_VOID_RETURN;
}
=== modified file 'sql/sql_lex.h'
--- a/sql/sql_lex.h 2009-01-15 10:48:31 +0000
+++ b/sql/sql_lex.h 2009-04-03 17:42:18 +0000
@@ -1176,6 +1176,22 @@ typedef struct st_lex : public Query_tab
bool escape_used;
+ /*
+ 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()
=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc 2009-03-25 17:50:42 +0000
+++ b/sql/sql_parse.cc 2009-04-03 17:42:18 +0000
@@ -2800,6 +2800,10 @@ mysql_execute_command(THD *thd)
if (res)
goto error;
+ if (!thd->locked_tables && lex->protect_against_global_read_lock &&
+ !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1)))
+ goto error;
+
if (!(res= open_and_lock_tables(thd, all_tables)))
{
if (lex->describe)
@@ -3660,6 +3664,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,
@@ -3677,6 +3684,7 @@ end_with_restore_list:
case SQLCOM_UPDATE_MULTI:
{
DBUG_ASSERT(first_table == all_tables && first_table != 0);
+
/* if we switched from normal update, rights are checked */
if (up_result != 2)
{
@@ -3686,6 +3694,10 @@ end_with_restore_list:
else
res= 0;
+ if (!thd->locked_tables &&
+ !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1)))
+ goto error;
+
res= mysql_multi_update_prepare(thd);
#ifdef HAVE_REPLICATION
@@ -3863,14 +3875,6 @@ 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,
@@ -4027,6 +4031,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);
@@ -4082,6 +4090,9 @@ end_with_restore_list:
goto error;
if (check_table_access(thd, LOCK_TABLES_ACL | SELECT_ACL, all_tables, 0))
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;
=== modified file 'sql/sql_yacc.yy'
--- a/sql/sql_yacc.yy 2009-02-12 14:36:43 +0000
+++ b/sql/sql_yacc.yy 2009-04-03 17:42:18 +0000
@@ -4522,6 +4522,7 @@ select_lock_type:
LEX *lex=Lex;
lex->current_select->set_lock_for_tables(TL_WRITE);
lex->safe_to_cache_query=0;
+ lex->protect_against_global_read_lock= TRUE;
}
| LOCK_SYM IN_SYM SHARE_SYM MODE_SYM
{
@@ -10058,8 +10059,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;
}
;
Attachment: [text/bzr-bundle] bzr/davi.arnaut@sun.com-20090403174218-2xoc09c60i673ou2.bundle
| Thread |
|---|
| • bzr commit into mysql-5.0-bugteam branch (davi:2726) Bug#43230 | Davi Arnaut | 3 Apr |
