2925 Konstantin Osipov 2009-11-20
Backport of:
revno: 2476.784.2
committer: davi@stripped
timestamp: Thu 2007-09-27 16:56:27 -0300
message:
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.
@ sql/lock.cc
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
used by this function and the same information is already on proc_info.
@ sql/sql_class.cc
Remove references to the THD::locked variable.
@ sql/sql_class.h
Remove the THD::locked variable.
@ sql/sql_show.cc
Remove references to THD:locked, state_info will now default to proc_info.
modified:
sql/lock.cc
sql/sql_class.cc
sql/sql_class.h
sql/sql_show.cc
2924 Magne Mahre 2009-11-20
Enable test cases for Bug#6063 and Bug#7088.
modified:
mysql-test/r/sp.result
mysql-test/t/sp.test
=== modified file 'sql/lock.cc'
--- a/sql/lock.cc 2009-10-14 16:37:38 +0000
+++ b/sql/lock.cc 2009-11-20 19:15:50 +0000
@@ -90,7 +90,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 *);
@@ -194,6 +193,60 @@ 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(MYSQL_LOCK *sql_lock)
+{
+ THR_LOCK_DATA **ldata, **ldata_end;
+ DBUG_ENTER("reset_lock_data");
+
+ /* 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;
+ }
+ DBUG_VOID_RETURN;
+}
+
+
+/**
+ Reset lock type in lock data and free.
+
+ @param mysql_lock Lock structures to reset.
+
+*/
+
+static void reset_lock_data_and_free(MYSQL_LOCK **mysql_lock)
+{
+ reset_lock_data(*mysql_lock);
+ my_free(*mysql_lock, MYF(0));
+ *mysql_lock= 0;
+}
+
+
MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **tables, uint count,
uint flags, bool *need_reopen)
{
@@ -224,16 +277,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;
}
}
@@ -248,9 +298,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;
}
@@ -261,14 +309,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_proc_info(thd, "Table lock");
DBUG_PRINT("info", ("thd->proc_info %s", thd->proc_info));
- thd->locked=1;
+ thd_proc_info(thd, "Locked");
/* 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));
@@ -281,22 +326,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))
{
@@ -304,23 +342,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_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;
@@ -863,8 +908,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= (uint) (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);
}
}
@@ -905,42 +949,6 @@ static MYSQL_LOCK *get_lock_data(THD *th
}
-/**
- Reset lock type in lock data.
-
- 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:
- -# Locking error in lock_external() due to InnoDB timeout.
- -# Locking error in get_lock_data() due to missing write permission.
- -# 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.
-
- @param sql_lock The MySQL lock.
-*/
-
-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;
- }
-}
-
-
/*****************************************************************************
Lock table based on the name.
This is used when we need total access to a closed, not open table
=== modified file 'sql/sql_class.cc'
--- a/sql/sql_class.cc 2009-11-19 23:48:08 +0000
+++ b/sql/sql_class.cc 2009-11-20 19:15:50 +0000
@@ -483,7 +483,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;
=== modified file 'sql/sql_class.h'
--- a/sql/sql_class.h 2009-11-19 23:48:08 +0000
+++ b/sql/sql_class.h 2009-11-20 19:15:50 +0000
@@ -1696,7 +1696,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;
/**
=== modified file 'sql/sql_show.cc'
--- a/sql/sql_show.cc 2009-11-19 23:48:08 +0000
+++ b/sql/sql_show.cc 2009-11-20 19:15:50 +0000
@@ -1707,8 +1707,6 @@ template class I_List<thread_info>;
static const char *thread_state_info(THD *tmp)
{
- if (tmp->locked)
- return "Locked";
#ifndef EMBEDDED_LIBRARY
if (tmp->net.reading_or_writing)
{
Attachment: [text/bzr-bundle] bzr/kostja@sun.com-20091120191550-o212devfco01602d.bundle
| Thread |
|---|
| • bzr push into mysql-5.6-next-mr branch (kostja:2924 to 2925) Bug#28870 | Konstantin Osipov | 20 Nov |
