From bafe24035dbf6d5452d7328ab45d6b43abe52d54 Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Mon, 7 Mar 2011 10:08:10 +0100 Subject: [PATCH] Bug #11764779 (former 57649) FLUSH TABLES under FLUSH TABLES WITH READ LOCK leads to assert failure. This assert was triggered if a statement tried up upgrade a metadata lock with an active FLUSH TABLE WITH READ LOCK. The assert checks that the connection already holds a global intention exclusive metadata lock. However, FLUSH TABLE WITH READ LOCK does not acquire this lock in order to be compatible with FLUSH TABLES WITH READ LOCK. Therefore any metadata lock upgrade caused the assert to be triggered. This patch fixes the problem by preventing metadata lock upgrade if the connection has an active FLUSH TABLE WITH READ LOCK. ER_TABLE_NOT_LOCKED_FOR_WRITE will instead be reported to the client. Test case added to flush.test. --- mysql-test/r/flush.result | 15 ++++++++++++ mysql-test/t/flush.test | 24 ++++++++++++++++++ sql/sql_base.cc | 51 ++++++++++++++++++++++++--------------- sql/sql_base.h | 4 +-- sql/sql_reload.cc | 20 ++++++++++++--- sql/sql_table.cc | 2 +- sql/sql_trigger.cc | 5 ++-- sql/sql_truncate.cc | 4 +-- 8 files changed, 95 insertions(+), 30 deletions(-) diff --git a/mysql-test/r/flush.result b/mysql-test/r/flush.result index b1e2e48eca8..bbfea2dade8 100644 --- a/mysql-test/r/flush.result +++ b/mysql-test/r/flush.result @@ -451,3 +451,18 @@ unlock tables; handler t1 close; # Cleanup. drop tables t1, t2; +# +# Bug#57649 FLUSH TABLES under FLUSH TABLES WITH READ LOCK leads +# to assert failure. +# +DROP TABLE IF EXISTS t1; +CREATE TABLE t1 (a INT); +FLUSH TABLES t1 WITH READ LOCK; +FLUSH TABLES; +ERROR HY000: Table 't1' was locked with a READ lock and can't be updated +CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW SET @a= 1; +ERROR HY000: Table 't1' was locked with a READ lock and can't be updated +ALTER TABLE t1 COMMENT 'test'; +ERROR HY000: Table 't1' was locked with a READ lock and can't be updated +UNLOCK TABLES; +DROP TABLE t1; diff --git a/mysql-test/t/flush.test b/mysql-test/t/flush.test index 944c9c43019..52ee6d2cf87 100644 --- a/mysql-test/t/flush.test +++ b/mysql-test/t/flush.test @@ -644,3 +644,27 @@ disconnect con2; --source include/wait_until_disconnected.inc connection default; drop tables t1, t2; + + +--echo # +--echo # Bug#57649 FLUSH TABLES under FLUSH TABLES WITH READ LOCK leads +--echo # to assert failure. +--echo # + +--disable_warnings +DROP TABLE IF EXISTS t1; +--enable_warnings + +CREATE TABLE t1 (a INT); +FLUSH TABLES t1 WITH READ LOCK; + +# All these triggered the assertion +--error ER_TABLE_NOT_LOCKED_FOR_WRITE +FLUSH TABLES; +--error ER_TABLE_NOT_LOCKED_FOR_WRITE +CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW SET @a= 1; +--error ER_TABLE_NOT_LOCKED_FOR_WRITE +ALTER TABLE t1 COMMENT 'test'; + +UNLOCK TABLES; +DROP TABLE t1; diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 8b6cc2caf10..7c020515f87 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -1026,7 +1026,7 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables, table_list= table_list->next_global) { /* A check that the table was locked for write is done by the caller. */ - TABLE *table= find_table_for_mdl_upgrade(thd->open_tables, table_list->db, + TABLE *table= find_table_for_mdl_upgrade(thd, table_list->db, table_list->table_name, TRUE); /* May return NULL if this table has already been closed via an alias. */ @@ -3120,22 +3120,26 @@ TABLE *find_locked_table(TABLE *list, const char *db, const char *table_name) lock from the list of open tables, emit error if no such table found. - @param list List of TABLE objects to be searched + @param thd Thread context @param db Database name. @param table_name Name of table. @param no_error Don't emit error if no suitable TABLE instance were found. + @note This function checks if the connection holds a global IX + metadata lock. If no such lock is found, it is not safe to + upgrade the lock and ER_TABLE_NOT_LOCKED_FOR_WRITE will be + reported. + @return Pointer to TABLE instance with MDL_SHARED_NO_WRITE, MDL_SHARED_NO_READ_WRITE, or MDL_EXCLUSIVE metadata lock, NULL otherwise. */ -TABLE *find_table_for_mdl_upgrade(TABLE *list, const char *db, - const char *table_name, - bool no_error) +TABLE *find_table_for_mdl_upgrade(THD *thd, const char *db, + const char *table_name, bool no_error) { - TABLE *tab= find_locked_table(list, db, table_name); + TABLE *tab= find_locked_table(thd->open_tables, db, table_name); if (!tab) { @@ -3143,19 +3147,29 @@ TABLE *find_table_for_mdl_upgrade(TABLE *list, const char *db, my_error(ER_TABLE_NOT_LOCKED, MYF(0), table_name); return NULL; } - else + + /* + It is not safe to upgrade the metadata lock without a global IX lock. + This can happen with FLUSH TABLES WITH READ LOCK as we in these + cases don't take a global IX lock in order to be compatible with + global read lock. + */ + if (!thd->mdl_context.is_lock_owner(MDL_key::GLOBAL, "", "", + MDL_INTENTION_EXCLUSIVE)) { - while (tab->mdl_ticket != NULL && - !tab->mdl_ticket->is_upgradable_or_exclusive() && - (tab= find_locked_table(tab->next, db, table_name))) - continue; - if (!tab) - { - if (!no_error) - my_error(ER_TABLE_NOT_LOCKED_FOR_WRITE, MYF(0), table_name); - return 0; - } + if (!no_error) + my_error(ER_TABLE_NOT_LOCKED_FOR_WRITE, MYF(0), table_name); + return NULL; } + + while (tab->mdl_ticket != NULL && + !tab->mdl_ticket->is_upgradable_or_exclusive() && + (tab= find_locked_table(tab->next, db, table_name))) + continue; + + if (!tab && !no_error) + my_error(ER_TABLE_NOT_LOCKED_FOR_WRITE, MYF(0), table_name); + return tab; } @@ -4653,8 +4667,7 @@ open_tables_check_upgradable_mdl(THD *thd, TABLE_LIST *tables_start, Note that find_table_for_mdl_upgrade() will report an error if no suitable ticket is found. */ - if (!find_table_for_mdl_upgrade(thd->open_tables, table->db, - table->table_name, FALSE)) + if (!find_table_for_mdl_upgrade(thd, table->db, table->table_name, false)) return TRUE; } } diff --git a/sql/sql_base.h b/sql/sql_base.h index 35fa04b3674..dc8320687fc 100644 --- a/sql/sql_base.h +++ b/sql/sql_base.h @@ -1,4 +1,4 @@ -/* Copyright 2006-2008 MySQL AB, 2008-2009 Sun Microsystems, Inc. +/* Copyright (c) 2006, 2011, Oracle and/or its affiliates. All rights reserved. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -290,7 +290,7 @@ bool tdc_open_view(THD *thd, TABLE_LIST *table_list, const char *alias, char *cache_key, uint cache_key_length, MEM_ROOT *mem_root, uint flags); void tdc_flush_unused_tables(); -TABLE *find_table_for_mdl_upgrade(TABLE *list, const char *db, +TABLE *find_table_for_mdl_upgrade(THD *thd, const char *db, const char *table_name, bool no_error); void mark_tmp_table_for_reuse(TABLE *table); diff --git a/sql/sql_reload.cc b/sql/sql_reload.cc index 0810459cb49..a09aa5511bd 100644 --- a/sql/sql_reload.cc +++ b/sql/sql_reload.cc @@ -1,4 +1,4 @@ -/* Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved. +/* Copyright (c) 2010, 2011, Oracle and/or its affiliates. All rights reserved. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -220,12 +220,26 @@ bool reload_acl_and_cache(THD *thd, unsigned long options, if (tables) { for (TABLE_LIST *t= tables; t; t= t->next_local) - if (!find_table_for_mdl_upgrade(thd->open_tables, t->db, - t->table_name, FALSE)) + if (!find_table_for_mdl_upgrade(thd, t->db, t->table_name, false)) return 1; } else { + /* + It is not safe to upgrade the metadata lock without GLOBAL IX lock. + This can happen with FLUSH TABLES WITH READ LOCK as we in these + cases don't take a GLOBAL IX lock in order to be compatible with + global read lock. + */ + if (thd->open_tables && + !thd->mdl_context.is_lock_owner(MDL_key::GLOBAL, "", "", + MDL_INTENTION_EXCLUSIVE)) + { + my_error(ER_TABLE_NOT_LOCKED_FOR_WRITE, MYF(0), + thd->open_tables->s->table_name.str); + return true; + } + for (TABLE *tab= thd->open_tables; tab; tab= tab->next) { if (! tab->mdl_ticket->is_upgradable_or_exclusive()) diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 2855bb0ec4a..677d2095a77 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -1917,7 +1917,7 @@ bool mysql_rm_table(THD *thd,TABLE_LIST *tables, my_bool if_exists, by parser) it is safe to cache pointer to the TABLE instances in its elements. */ - table->table= find_table_for_mdl_upgrade(thd->open_tables, table->db, + table->table= find_table_for_mdl_upgrade(thd, table->db, table->table_name, false); if (!table->table) DBUG_RETURN(true); diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc index c2e3cd9944a..e986f6d7e92 100644 --- a/sql/sql_trigger.cc +++ b/sql/sql_trigger.cc @@ -1,4 +1,4 @@ -/* Copyright (C) 2004-2005 MySQL AB, 2008-2009 Sun Microsystems, Inc +/* Copyright (c) 2004, 2011, Oracle and/or its affiliates. All rights reserved. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -467,8 +467,7 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create) if (thd->locked_tables_mode) { /* Under LOCK TABLES we must only accept write locked tables. */ - if (!(tables->table= find_table_for_mdl_upgrade(thd->open_tables, - tables->db, + if (!(tables->table= find_table_for_mdl_upgrade(thd, tables->db, tables->table_name, FALSE))) goto end; diff --git a/sql/sql_truncate.cc b/sql/sql_truncate.cc index 909c6a08b67..67ed608f114 100644 --- a/sql/sql_truncate.cc +++ b/sql/sql_truncate.cc @@ -1,4 +1,4 @@ -/* Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved. +/* Copyright (c) 2010, 2011, Oracle and/or its affiliates. All rights reserved. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -327,7 +327,7 @@ bool Truncate_statement::lock_table(THD *thd, TABLE_LIST *table_ref, */ if (thd->locked_tables_mode) { - if (!(table= find_table_for_mdl_upgrade(thd->open_tables, table_ref->db, + if (!(table= find_table_for_mdl_upgrade(thd, table_ref->db, table_ref->table_name, FALSE))) DBUG_RETURN(TRUE);