mirror/mariadb
1
0
Fork 0
mirror of https://github.com/MariaDB/server.git synced 2025-01-16 03:52:35 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

Bug#32395 Alter table under a impending global read lock causes a server crash

Browse source 
The problem is that some DDL statements (ALTER TABLE, CREATE
TRIGGER, FLUSH TABLES, ...) when under LOCK TABLES need to
momentarily drop the lock, reopen the table and grab the write
lock again (using reopen_tables). When grabbing the lock again,
reopen_tables doesn't pass a flag to mysql_lock_tables in
order to ignore the impending global read lock, which causes a
assertion because LOCK_open is being hold. Also dropping the
lock must not signal to any threads that the table has been
relinquished (related to the locking/flushing protocol).

The solution is to correct the way the table is reopenned
and the locks grabbed. When reopening the table and under
LOCK TABLES, the table version should be set to 0 so other
threads have to wait for the table. When grabbing the lock,
any other flush should be ignored because it's theoretically
a atomic operation. The chosen solution also fixes a potential
discrepancy between binlog and GRL (global read lock) because
table placeholders were being ignored, now a FLUSH TABLES WITH
READ LOCK will properly for table with open placeholders.

It's also important to mention that this patch doesn't fix
a potential deadlock if one uses two GRLs under LOCK TABLES
concurrently.


mysql-test/r/lock_multi.result:
  Add test case result for Bug#32395
mysql-test/r/trigger_notembedded.result:
  Add test case result for Bug#32395
mysql-test/t/lock_multi.test:
  Add test case for Bug#32395
mysql-test/t/trigger_notembedded.test:
  Enable test case for Bug#32395
sql/ha_ndbcluster.cc:
  Update close_cached_tables usage.
sql/ha_ndbcluster_binlog.cc:
  Update close_cached_tables usage.
sql/mysql_priv.h:
  Update close_cache_tables prototype.
sql/set_var.cc:
  Update close_cached_tables usage and set flag to wait for
  tables with placeholders. This is one of the places where
  a GRL can be obtained.
sql/sql_base.cc:
  Preserve old version for write locked tables and ignore
  pending flushes and update close_cache_tables to take
  into account name locked tables.
sql/sql_parse.cc:
  Update close_cached_tables usage and pass flag so that
  name locked tables are waited for.
sql/sql_table.cc:
  Protect the table against a impending GRL if under LOCK TABLES.
This commit is contained in:
unknown 2007-12-12 19:44:14 -02:00
parent ede6f50a6a
commit e5a397e28f
11 changed files with 206 additions and 41 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

30
mysql-test/r/lock_multi.result
View file

@ -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

14
mysql-test/r/trigger_notembedded.result
View file

@ -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.

98
mysql-test/t/lock_multi.test
View file

@ -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

6
mysql-test/t/trigger_notembedded.test
View file

@ -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.

4
sql/ha_ndbcluster.cc
View file

@ -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 */

12
sql/ha_ndbcluster_binlog.cc
View file

@ -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;

3
sql/mysql_priv.h
View file

@ -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);

2
sql/set_var.cc
View file

@ -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))

69
sql/sql_base.cc
View file

@ -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, &not_used)))
flags, &not_used)))
{
thd->locked_tables=mysql_lock_merge(thd->locked_tables,lock);
}

7
sql/sql_parse.cc
View file

@ -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)

2
sql/sql_table.cc
View file

@ -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;