Below is the list of changes that have just been committed into a local
5.2 repository of davi. When davi does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
ChangeSet@stripped, 2007-09-10 19:22:07-03:00, davi@stripped +11 -0
Bug#28870 check that table locks are released/reset
The problem is that some mysql_lock_tables error paths are not resetting the
tables lock type back to TL_UNLOCK. If the lock types are not reset properly,
a table might be returned to the table cache with wrong lock_type.
The proposed fix is to ensure that the tables lock type is always properly
reset when mysql_lock_tables fails. This is a incompatible change with respect
to the process state information.
mysql-test/r/sp-threads.result@stripped, 2007-09-10 19:22:03-03:00, davi@stripped +1 -1
Update process state information for table locking.
mysql-test/suite/funcs_1/r/a_processlist_val_no_prot.result@stripped, 2007-09-10 19:22:03-03:00, davi@stripped +2 -2
Update process state information for table locking.
mysql-test/suite/funcs_1/r/b_processlist_val_ps.result@stripped, 2007-09-10 19:22:03-03:00, davi@stripped +2 -2
Update process state information for table locking.
mysql-test/t/insert_notembedded.test@stripped, 2007-09-10 19:22:03-03:00, davi@stripped +1 -1
Update process state information for table locking.
mysql-test/t/lock_multi.test@stripped, 2007-09-10 19:22:03-03:00, davi@stripped +11 -11
Update process state information for table locking.
mysql-test/t/sp_notembedded.test@stripped, 2007-09-10 19:22:03-03:00, davi@stripped +1 -1
Update process state information for table locking.
mysql-test/t/status.test@stripped, 2007-09-10 19:22:03-03:00, davi@stripped +1 -1
Update process state information for table locking.
sql/lock.cc@stripped, 2007-09-10 19:22:03-03:00, davi@stripped +59 -72
Merge mysql_lock_tables cleanup sequence and the reset_lock_data
function into a single function and take steps to ensure it is
always called for each error exit path. Also remove references
to the redundant THD::locked variable which was almost exclusively
by this function and the same information is already on proc_info.
sql/sql_class.cc@stripped, 2007-09-10 19:22:04-03:00, davi@stripped +1 -1
Remove references to the THD::locked variable.
sql/sql_class.h@stripped, 2007-09-10 19:22:04-03:00, davi@stripped +1 -1
Remove the THD::locked variable.
sql/sql_show.cc@stripped, 2007-09-10 19:22:04-03:00, davi@stripped +2 -4
Remove references to THD:locked, state_info will now default to proc_info.
diff -Nrup a/mysql-test/r/sp-threads.result b/mysql-test/r/sp-threads.result
--- a/mysql-test/r/sp-threads.result 2006-10-04 11:33:24 -03:00
+++ b/mysql-test/r/sp-threads.result 2007-09-10 19:22:03 -03:00
@@ -35,7 +35,7 @@ call bug9486();
show processlist;
Id User Host db Command Time State Info
# root localhost test Sleep # NULL
-# root localhost test Query # Locked update t1, t2 set val= 1 where id1=id2
+# root localhost test Query # Table lock update t1, t2 set val= 1 where id1=id2
# root localhost test Query # NULL show processlist
# root localhost test Sleep # NULL
unlock tables;
diff -Nrup a/mysql-test/suite/funcs_1/r/a_processlist_val_no_prot.result b/mysql-test/suite/funcs_1/r/a_processlist_val_no_prot.result
--- a/mysql-test/suite/funcs_1/r/a_processlist_val_no_prot.result 2007-08-15 16:46:41 -03:00
+++ b/mysql-test/suite/funcs_1/r/a_processlist_val_no_prot.result 2007-09-10 19:22:03 -03:00
@@ -155,12 +155,12 @@ SELECT COUNT(*) FROM test.t1;
# Sleep some time
SELECT * FROM INFORMATION_SCHEMA.PROCESSLIST;
ID USER HOST DB COMMAND TIME STATE INFO
-<ID> ddicttestuser1 localhost information_schema Query <TIME> Locked SELECT COUNT(*) FROM test.t1
+<ID> ddicttestuser1 localhost information_schema Query <TIME> Table lock SELECT COUNT(*) FROM test.t1
<ID> root localhost information_schema Query <TIME> preparing SELECT * FROM INFORMATION_SCHEMA.PROCESSLIST
SHOW FULL PROCESSLIST;
Id User Host db Command Time State Info
<ID> root localhost information_schema Query <TIME> NULL SHOW FULL PROCESSLIST
-<ID> ddicttestuser1 localhost information_schema Query <TIME> Locked SELECT COUNT(*) FROM test.t1
+<ID> ddicttestuser1 localhost information_schema Query <TIME> Table lock SELECT COUNT(*) FROM test.t1
UNLOCK TABLES;
----- switch to connection ddicttestuser1 (user = ddicttestuser1) -----
diff -Nrup a/mysql-test/suite/funcs_1/r/b_processlist_val_ps.result b/mysql-test/suite/funcs_1/r/b_processlist_val_ps.result
--- a/mysql-test/suite/funcs_1/r/b_processlist_val_ps.result 2007-08-15 16:46:41 -03:00
+++ b/mysql-test/suite/funcs_1/r/b_processlist_val_ps.result 2007-09-10 19:22:03 -03:00
@@ -155,12 +155,12 @@ SELECT COUNT(*) FROM test.t1;
# Sleep some time
SELECT * FROM INFORMATION_SCHEMA.PROCESSLIST;
ID USER HOST DB COMMAND TIME STATE INFO
-<ID> ddicttestuser1 localhost information_schema Query <TIME> Locked SELECT COUNT(*) FROM test.t1
+<ID> ddicttestuser1 localhost information_schema Query <TIME> Table lock SELECT COUNT(*) FROM test.t1
<ID> root localhost information_schema Execute <TIME> preparing SELECT * FROM INFORMATION_SCHEMA.PROCESSLIST
SHOW FULL PROCESSLIST;
Id User Host db Command Time State Info
<ID> root localhost information_schema Query <TIME> NULL SHOW FULL PROCESSLIST
-<ID> ddicttestuser1 localhost information_schema Query <TIME> Locked SELECT COUNT(*) FROM test.t1
+<ID> ddicttestuser1 localhost information_schema Query <TIME> Table lock SELECT COUNT(*) FROM test.t1
UNLOCK TABLES;
----- switch to connection ddicttestuser1 (user = ddicttestuser1) -----
diff -Nrup a/mysql-test/t/insert_notembedded.test b/mysql-test/t/insert_notembedded.test
--- a/mysql-test/t/insert_notembedded.test 2007-09-06 13:22:31 -03:00
+++ b/mysql-test/t/insert_notembedded.test 2007-09-10 19:22:03 -03:00
@@ -174,7 +174,7 @@ connection default;
# we must wait till the insert opens and locks the table
let $wait_condition=
select count(*) = 1 from information_schema.processlist
- where state = "Locked" and id = $ID;
+ where state = "Table lock" and id = $ID;
--source include/wait_condition.inc
connect (select,localhost,root,,);
--echo connection: select
diff -Nrup a/mysql-test/t/lock_multi.test b/mysql-test/t/lock_multi.test
--- a/mysql-test/t/lock_multi.test 2007-08-17 11:34:08 -03:00
+++ b/mysql-test/t/lock_multi.test 2007-09-10 19:22:03 -03:00
@@ -18,13 +18,13 @@ send update low_priority t1 set n = 4;
connection reader;
let $wait_condition=
select count(*) = 1 from information_schema.processlist
- where state = "Locked" and info = "update low_priority t1 set n = 4";
+ where state = "Table lock" and info = "update low_priority t1 set n = 4";
--source include/wait_condition.inc
send select n from t1;
connection locker;
let $wait_condition=
select count(*) = 1 from information_schema.processlist
- where state = "Locked" and info = "select n from t1";
+ where state = "Table lock" and info = "select n from t1";
--source include/wait_condition.inc
unlock tables;
connection writer;
@@ -42,7 +42,7 @@ send update low_priority t1 set n = 4;
connection reader;
let $wait_condition=
select count(*) = 1 from information_schema.processlist
- where state = "Locked" and info = "update low_priority t1 set n = 4";
+ where state = "Table lock" and info = "update low_priority t1 set n = 4";
--source include/wait_condition.inc
select n from t1;
connection locker;
@@ -85,7 +85,7 @@ send insert t1 select * from t2;
connection locker;
let $wait_condition=
select count(*) = 1 from information_schema.processlist
- where state = "Locked" and info = "insert t1 select * from t2";
+ where state = "Table lock" and info = "insert t1 select * from t2";
--source include/wait_condition.inc
drop table t2;
connection reader;
@@ -107,7 +107,7 @@ send insert t1 select * from t2;
connection locker;
let $wait_condition=
select count(*) = 1 from information_schema.processlist
- where state = "Locked" and info = "insert t1 select * from t2";
+ where state = "Table lock" and info = "insert t1 select * from t2";
--source include/wait_condition.inc
drop table t2;
connection reader;
@@ -150,7 +150,7 @@ send SELECT user.Select_priv FROM user,
connection locker;
let $wait_condition=
select count(*) = 1 from information_schema.processlist
- where state = "Locked" and info =
+ where state = "Table lock" 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.
@@ -276,13 +276,13 @@ send alter table t1 auto_increment=0;
connection reader;
let $wait_condition=
select count(*) = 1 from information_schema.processlist
- where state = "Locked" and info = "alter table t1 auto_increment=0";
+ where state = "Table lock" and info = "alter table t1 auto_increment=0";
--source include/wait_condition.inc
send alter table t1 auto_increment=0;
connection locker;
let $wait_condition=
select count(*) = 2 from information_schema.processlist
- where state = "Locked" and info = "alter table t1 auto_increment=0";
+ where state = "Table lock" and info = "alter table t1 auto_increment=0";
--source include/wait_condition.inc
unlock tables;
connection writer;
@@ -307,15 +307,15 @@ connection writer;
connection reader;
let $wait_condition=
select count(*) = 1 from information_schema.processlist
- where state = "Locked" and info = "update t1 set i= 10";
+ where state = "Table lock" and info = "update t1 set i= 10";
--source include/wait_condition.inc
--send select * from t1;
connection default;
let $wait_condition=
select count(*) = 1 from information_schema.processlist
- where state = "Locked" and info = "select * from t1";
+ where state = "Table lock" and info = "select * from t1";
--source include/wait_condition.inc
-let $ID= `select id from information_schema.processlist where state = "Locked" and info = "update t1 set i= 10"`;
+let $ID= `select id from information_schema.processlist where state = "Table lock" and info = "update t1 set i= 10"`;
--replace_result $ID ID
eval kill query $ID;
connection reader;
diff -Nrup a/mysql-test/t/sp_notembedded.test b/mysql-test/t/sp_notembedded.test
--- a/mysql-test/t/sp_notembedded.test 2007-09-05 15:02:59 -03:00
+++ b/mysql-test/t/sp_notembedded.test 2007-09-10 19:22:03 -03:00
@@ -331,7 +331,7 @@ set session low_priority_updates=on;
connection rl_wait;
let $wait_condition=
select count(*) = 1 from information_schema.processlist
- where state = "Locked" and
+ where state = "Table lock" and
info = "update t1 set value='updated' where value='old'";
--source include/wait_condition.inc
diff -Nrup a/mysql-test/t/status.test b/mysql-test/t/status.test
--- a/mysql-test/t/status.test 2007-08-25 06:08:12 -03:00
+++ b/mysql-test/t/status.test 2007-09-10 19:22:03 -03:00
@@ -48,7 +48,7 @@ update t1 set n = 3;
connection con2;
# wait for the other query to start executing
-let $wait_condition= select 1 from INFORMATION_SCHEMA.PROCESSLIST where ID = $ID and STATE = "Locked";
+let $wait_condition= select 1 from INFORMATION_SCHEMA.PROCESSLIST where ID = $ID and STATE = "Table lock";
# Immediate = 14 + $wait_condition_reps ($wait_timeout is 0, so no extra select
# is done inside wait_condition.inc)
--source include/wait_condition.inc
diff -Nrup a/sql/lock.cc b/sql/lock.cc
--- a/sql/lock.cc 2007-08-29 01:58:51 -03:00
+++ b/sql/lock.cc 2007-09-10 19:22:03 -03:00
@@ -87,7 +87,6 @@ extern HASH open_cache;
static MYSQL_LOCK *get_lock_data(THD *thd, TABLE **table,uint count,
uint flags, TABLE **write_locked);
-static void reset_lock_data(MYSQL_LOCK *sql_lock);
static int lock_external(THD *thd, TABLE **table,uint count);
static int unlock_external(THD *thd, TABLE **table,uint count);
static void print_lock_error(int error, const char *);
@@ -191,6 +190,45 @@ int mysql_lock_tables_check(THD *thd, TA
DBUG_RETURN(0);
}
+
+/**
+ Reset lock type in lock data and free.
+
+ @param mysql_lock Lock structures to reset.
+
+ @note After a locking error we want to quit the locking of the table(s).
+ The test case in the bug report for Bug #18544 has the following
+ cases: 1. Locking error in lock_external() due to InnoDB timeout.
+ 2. Locking error in get_lock_data() due to missing write permission.
+ 3. Locking error in wait_if_global_read_lock() due to lock conflict.
+
+ @note In all these cases we have already set the lock type into the lock
+ data of the open table(s). If the table(s) are in the open table
+ cache, they could be reused with the non-zero lock type set. This
+ could lead to ignoring a different lock type with the next lock.
+
+ @note Clear the lock type of all lock data. This ensures that the next
+ lock request will set its lock type properly.
+*/
+
+static void reset_lock_data_and_free(MYSQL_LOCK **mysql_lock)
+{
+ MYSQL_LOCK *sql_lock= *mysql_lock;
+ THR_LOCK_DATA **ldata, **ldata_end;
+
+ /* Clear the lock type of all lock data to avoid reusage. */
+ for (ldata= sql_lock->locks, ldata_end= ldata + sql_lock->lock_count;
+ ldata < ldata_end;
+ ldata++)
+ {
+ /* Reset lock type. */
+ (*ldata)->type= TL_UNLOCK;
+ }
+ my_free((uchar*) sql_lock, MYF(0));
+ *mysql_lock= 0;
+}
+
+
MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **tables, uint count,
uint flags, bool *need_reopen)
{
@@ -221,16 +259,13 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd,
if (wait_if_global_read_lock(thd, 1, 1))
{
/* Clear the lock type of all lock data to avoid reusage. */
- reset_lock_data(sql_lock);
- my_free((uchar*) sql_lock,MYF(0));
- sql_lock=0;
+ reset_lock_data_and_free(&sql_lock);
break;
}
if (thd->version != refresh_version)
{
/* Clear the lock type of all lock data to avoid reusage. */
- reset_lock_data(sql_lock);
- my_free((uchar*) sql_lock,MYF(0));
+ reset_lock_data_and_free(&sql_lock);
goto retry;
}
}
@@ -245,9 +280,7 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd,
Someone has issued SET GLOBAL READ_ONLY=1 and we want a write lock.
We do not wait for READ_ONLY=0, and fail.
*/
- reset_lock_data(sql_lock);
- my_free((uchar*) sql_lock, MYF(0));
- sql_lock=0;
+ reset_lock_data_and_free(&sql_lock);
my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only");
break;
}
@@ -258,14 +291,11 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd,
sql_lock->table_count))
{
/* Clear the lock type of all lock data to avoid reusage. */
- reset_lock_data(sql_lock);
- my_free((uchar*) sql_lock,MYF(0));
- sql_lock=0;
+ reset_lock_data_and_free(&sql_lock);
break;
}
THD_SET_PROC_INFO(thd, "Table lock");
DBUG_PRINT("info", ("thd->proc_info %s", thd->proc_info));
- thd->locked=1;
/* Copy the lock data array. thr_multi_lock() reorders its contens. */
memcpy(sql_lock->locks + sql_lock->lock_count, sql_lock->locks,
sql_lock->lock_count * sizeof(*sql_lock->locks));
@@ -278,22 +308,15 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd,
{
if (sql_lock->table_count)
VOID(unlock_external(thd, sql_lock->table, sql_lock->table_count));
+ reset_lock_data_and_free(&sql_lock);
my_error(rc, MYF(0));
- my_free((uchar*) sql_lock,MYF(0));
- sql_lock= 0;
break;
}
else if (rc == 1) /* aborted */
{
- /*
- reset_lock_data is required here. If thr_multi_lock fails it
- resets lock type for tables, which were locked before (and
- including) one that caused error. Lock type for other tables
- preserved.
- */
- reset_lock_data(sql_lock);
thd->some_tables_deleted=1; // Try again
sql_lock->lock_count= 0; // Locks are already freed
+ // Fall through: unlock, reset lock data, free and retry
}
else if (!thd->some_tables_deleted || (flags & MYSQL_LOCK_IGNORE_FLUSH))
{
@@ -301,23 +324,30 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd,
Thread was killed or lock aborted. Let upper level close all
used tables and retry or give error.
*/
- thd->locked=0;
break;
}
else if (!thd->open_tables)
{
// Only using temporary tables, no need to unlock
thd->some_tables_deleted=0;
- thd->locked=0;
break;
}
THD_SET_PROC_INFO(thd, 0);
- /* some table was altered or deleted. reopen tables marked deleted */
- mysql_unlock_tables(thd,sql_lock);
- thd->locked=0;
+ /* going to retry, unlock all tables */
+ if (sql_lock->lock_count)
+ thr_multi_unlock(sql_lock->locks, sql_lock->lock_count);
+
+ if (sql_lock->table_count)
+ VOID(unlock_external(thd, sql_lock->table, sql_lock->table_count));
+
+ /*
+ If thr_multi_lock fails it resets lock type for tables, which
+ were locked before (and including) one that caused error. Lock
+ type for other tables preserved.
+ */
+ reset_lock_data_and_free(&sql_lock);
retry:
- sql_lock=0;
if (flags & MYSQL_LOCK_NOTIFY_IF_NEED_REOPEN)
{
*need_reopen= TRUE;
@@ -868,8 +898,7 @@ static MYSQL_LOCK *get_lock_data(THD *th
my_error(ER_OPEN_AS_READONLY,MYF(0),table->alias);
/* Clear the lock type of the lock data that are stored already. */
sql_lock->lock_count= locks - sql_lock->locks;
- reset_lock_data(sql_lock);
- my_free((uchar*) sql_lock,MYF(0));
+ reset_lock_data_and_free(&sql_lock);
DBUG_RETURN(0);
}
}
@@ -890,48 +919,6 @@ static MYSQL_LOCK *get_lock_data(THD *th
(*org_locks)->debug_print_param= (void *) table;
}
DBUG_RETURN(sql_lock);
-}
-
-
-/*
- Reset lock type in lock data.
-
- SYNOPSIS
- reset_lock_data()
- sql_lock The MySQL lock.
-
- DESCRIPTION
-
- After a locking error we want to quit the locking of the table(s).
- The test case in the bug report for Bug #18544 has the following
- cases: 1. Locking error in lock_external() due to InnoDB timeout.
- 2. Locking error in get_lock_data() due to missing write permission.
- 3. Locking error in wait_if_global_read_lock() due to lock conflict.
-
- In all these cases we have already set the lock type into the lock
- data of the open table(s). If the table(s) are in the open table
- cache, they could be reused with the non-zero lock type set. This
- could lead to ignoring a different lock type with the next lock.
-
- Clear the lock type of all lock data. This ensures that the next
- lock request will set its lock type properly.
-
- RETURN
- void
-*/
-
-static void reset_lock_data(MYSQL_LOCK *sql_lock)
-{
- THR_LOCK_DATA **ldata;
- THR_LOCK_DATA **ldata_end;
-
- for (ldata= sql_lock->locks, ldata_end= ldata + sql_lock->lock_count;
- ldata < ldata_end;
- ldata++)
- {
- /* Reset lock type. */
- (*ldata)->type= TL_UNLOCK;
- }
}
diff -Nrup a/sql/sql_class.cc b/sql/sql_class.cc
--- a/sql/sql_class.cc 2007-09-04 11:52:01 -03:00
+++ b/sql/sql_class.cc 2007-09-10 19:22:04 -03:00
@@ -390,7 +390,7 @@ THD::THD()
catalog= (char*)"std"; // the only catalog we have for now
main_security_ctx.init();
security_ctx= &main_security_ctx;
- locked=some_tables_deleted=no_errors=password= 0;
+ some_tables_deleted=no_errors=password= 0;
query_start_used= 0;
count_cuted_fields= CHECK_FIELD_IGNORE;
killed= NOT_KILLED;
diff -Nrup a/sql/sql_class.h b/sql/sql_class.h
--- a/sql/sql_class.h 2007-09-04 11:52:01 -03:00
+++ b/sql/sql_class.h 2007-09-10 19:22:04 -03:00
@@ -1457,7 +1457,7 @@ public:
bool slave_thread, one_shot_set;
/* tells if current statement should binlog row-based(1) or stmt-based(0) */
bool current_stmt_binlog_row_based;
- bool locked, some_tables_deleted;
+ bool some_tables_deleted;
bool last_cuted_field;
bool no_errors, password;
/**
diff -Nrup a/sql/sql_show.cc b/sql/sql_show.cc
--- a/sql/sql_show.cc 2007-08-30 19:49:11 -03:00
+++ b/sql/sql_show.cc 2007-09-10 19:22:04 -03:00
@@ -1699,8 +1699,7 @@ void mysqld_list_processes(THD *thd,cons
pthread_mutex_lock(&mysys_var->mutex);
thd_info->proc_info= (char*) (tmp->killed == THD::KILL_CONNECTION? "Killed" : 0);
#ifndef EMBEDDED_LIBRARY
- thd_info->state_info= (char*) (tmp->locked ? "Locked" :
- tmp->net.reading_or_writing ?
+ thd_info->state_info= (char*) (tmp->net.reading_or_writing ?
(tmp->net.reading_or_writing == 2 ?
"Writing to net" :
thd_info->command == COM_SLEEP ? "" :
@@ -1826,8 +1825,7 @@ int fill_schema_processlist(THD* thd, TA
now - tmp->start_time : 0), TRUE);
/* STATE */
#ifndef EMBEDDED_LIBRARY
- val= (char*) (tmp->locked ? "Locked" :
- tmp->net.reading_or_writing ?
+ val= (char*) (tmp->net.reading_or_writing ?
(tmp->net.reading_or_writing == 2 ?
"Writing to net" :
tmp->command == COM_SLEEP ? "" :
