From c290b26c0eb3010eb7d605d119015eb6953060f9 Mon Sep 17 00:00:00 2001 From: "davi@moksha.local" <> Date: Wed, 15 Aug 2007 12:13:34 -0300 Subject: [PATCH 1/3] Bug#25856 (HANDLER table OPEN in one connection lock DROP TABLE in another one) mysql_ha_open calls mysql_ha_close on the error path (unsupported) to close the (opened) table before inserting it into the tables hash list handler_tables_hash) but mysql_ha_close only closes tables which are on the hash list, causing the table to be left open and locked. This change moves the table close logic into a separate function that is always called on the error path of mysql_ha_open or on a normal handler close (mysql_ha_close). --- mysql-test/r/handler.result | 7 ++++ mysql-test/t/handler.test | 14 +++++++ sql/sql_handler.cc | 80 ++++++++++++++++++++++--------------- 3 files changed, 68 insertions(+), 33 deletions(-) diff --git a/mysql-test/r/handler.result b/mysql-test/r/handler.result index 85cf47b5806..5e123df9103 100644 --- a/mysql-test/r/handler.result +++ b/mysql-test/r/handler.result @@ -482,3 +482,10 @@ ERROR 42S02: Table 'test.t1' doesn't exist drop table if exists t1; Warnings: Note 1051 Unknown table 't1' +drop table if exists t1; +create table t1 (a int) ENGINE=MEMORY; +--> client 2 +handler t1 open; +ERROR HY000: Table storage engine for 't1' doesn't have this option +--> client 1 +drop table t1; diff --git a/mysql-test/t/handler.test b/mysql-test/t/handler.test index bf18b8da941..4edd5b5ec32 100644 --- a/mysql-test/t/handler.test +++ b/mysql-test/t/handler.test @@ -427,3 +427,17 @@ select * from t1; # Just to be sure and not confuse the next test case writer. drop table if exists t1; +# +# Bug#25856 - HANDLER table OPEN in one connection lock DROP TABLE in another one +# +--disable_warnings +drop table if exists t1; +--enable_warnings +create table t1 (a int) ENGINE=MEMORY; +--echo --> client 2 +connection con2; +--error 1031 +handler t1 open; +--echo --> client 1 +connection default; +drop table t1; diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc index e1318aa2736..e00d9110378 100644 --- a/sql/sql_handler.cc +++ b/sql/sql_handler.cc @@ -119,6 +119,44 @@ static void mysql_ha_hash_free(TABLE_LIST *tables) my_free((char*) tables, MYF(0)); } +/** + * Close a HANDLER table. + * + * @param thd Thread identifier. + * @param A list of tables with the first entry to close. + * + * @note Though this function takes a list of tables, only the first list entry + * will be closed. + * @note Broadcasts refresh if it closed the table. + */ + +static void mysql_ha_close_table(THD *thd, TABLE_LIST *tables) +{ + TABLE **table_ptr; + + /* + Though we could take the table pointer from hash_tables->table, + we must follow the thd->handler_tables chain anyway, as we need the + address of the 'next' pointer referencing this table + for close_thread_table(). + */ + for (table_ptr= &(thd->handler_tables); + *table_ptr && (*table_ptr != tables->table); + table_ptr= &(*table_ptr)->next) + ; + + if (*table_ptr) + { + (*table_ptr)->file->ha_index_or_rnd_end(); + VOID(pthread_mutex_lock(&LOCK_open)); + if (close_thread_table(thd, table_ptr)) + { + /* Tell threads waiting for refresh that something has happened */ + broadcast_refresh(); + } + VOID(pthread_mutex_unlock(&LOCK_open)); + } +} /* Open a HANDLER table. @@ -145,7 +183,7 @@ static void mysql_ha_hash_free(TABLE_LIST *tables) bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen) { - TABLE_LIST *hash_tables; + TABLE_LIST *hash_tables = NULL; char *db, *name, *alias; uint dblen, namelen, aliaslen, counter; int error; @@ -197,7 +235,6 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen) { if (! reopen) my_error(ER_ILLEGAL_HA, MYF(0), tables->alias); - mysql_ha_close(thd, tables); goto err; } @@ -225,11 +262,7 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen) /* add to hash */ if (my_hash_insert(&thd->handler_tables_hash, (byte*) hash_tables)) - { - my_free((char*) hash_tables, MYF(0)); - mysql_ha_close(thd, tables); goto err; - } } if (! reopen) @@ -238,13 +271,17 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen) DBUG_RETURN(FALSE); err: + if (hash_tables) + my_free((char*) hash_tables, MYF(0)); + if (tables->table) + mysql_ha_close_table(thd, tables); DBUG_PRINT("exit",("ERROR")); DBUG_RETURN(TRUE); } /* - Close a HANDLER table. + Close a HANDLER table by alias or table name SYNOPSIS mysql_ha_close() @@ -252,9 +289,8 @@ err: tables A list of tables with the first entry to close. DESCRIPTION - Though this function takes a list of tables, only the first list entry - will be closed. - Broadcasts refresh if it closed the table. + Closes the table that is associated (on the handler tables hash) with the + name (table->alias) of the specified table. RETURN FALSE ok @@ -264,7 +300,6 @@ err: bool mysql_ha_close(THD *thd, TABLE_LIST *tables) { TABLE_LIST *hash_tables; - TABLE **table_ptr; DBUG_ENTER("mysql_ha_close"); DBUG_PRINT("enter",("'%s'.'%s' as '%s'", tables->db, tables->table_name, tables->alias)); @@ -273,28 +308,7 @@ bool mysql_ha_close(THD *thd, TABLE_LIST *tables) (byte*) tables->alias, strlen(tables->alias) + 1))) { - /* - Though we could take the table pointer from hash_tables->table, - we must follow the thd->handler_tables chain anyway, as we need the - address of the 'next' pointer referencing this table - for close_thread_table(). - */ - for (table_ptr= &(thd->handler_tables); - *table_ptr && (*table_ptr != hash_tables->table); - table_ptr= &(*table_ptr)->next) - ; - - if (*table_ptr) - { - (*table_ptr)->file->ha_index_or_rnd_end(); - VOID(pthread_mutex_lock(&LOCK_open)); - if (close_thread_table(thd, table_ptr)) - { - /* Tell threads waiting for refresh that something has happened */ - broadcast_refresh(); - } - VOID(pthread_mutex_unlock(&LOCK_open)); - } + mysql_ha_close_table(thd, hash_tables); hash_delete(&thd->handler_tables_hash, (byte*) hash_tables); } else From d4aad350957bb8f75ad5c60084ded86f063bdc28 Mon Sep 17 00:00:00 2001 From: "davi@moksha.local" <> Date: Wed, 15 Aug 2007 17:12:09 -0300 Subject: [PATCH 2/3] Rework doxygen documentation for the function mysql_ha_close_table. --- sql/sql_handler.cc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc index e00d9110378..4ff91bd40a6 100644 --- a/sql/sql_handler.cc +++ b/sql/sql_handler.cc @@ -120,15 +120,15 @@ static void mysql_ha_hash_free(TABLE_LIST *tables) } /** - * Close a HANDLER table. - * - * @param thd Thread identifier. - * @param A list of tables with the first entry to close. - * - * @note Though this function takes a list of tables, only the first list entry - * will be closed. - * @note Broadcasts refresh if it closed the table. - */ + Close a HANDLER table. + + @param thd Thread identifier. + @param tables A list of tables with the first entry to close. + + @note Though this function takes a list of tables, only the first list entry + will be closed. + @note Broadcasts refresh if it closed the table. +*/ static void mysql_ha_close_table(THD *thd, TABLE_LIST *tables) { From d6692280061d9391e5cae67fcf11f87dc6fd8a13 Mon Sep 17 00:00:00 2001 From: "davi@moksha.local" <> Date: Thu, 16 Aug 2007 14:51:37 -0300 Subject: [PATCH 3/3] Bug#29936 (Stored Procedure DML ignores low_priority_updates setting) This is a follow up for the patch for Bug#26162 "Trigger DML ignores low_priority_updates setting", where the stored procedure ignores the session setting of low_priority_updates. For every table open operation with default write (TL_WRITE_DEFAULT) lock_type, downgrade the lock type to the session setting of low_priority_updates. --- sql/lock.cc | 1 + sql/sql_base.cc | 9 ++------- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/sql/lock.cc b/sql/lock.cc index f730ac56d35..0036d0aef77 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -730,6 +730,7 @@ static MYSQL_LOCK *get_lock_data(THD *thd, TABLE **table_ptr, uint count, if ((table=table_ptr[i])->s->tmp_table == NON_TRANSACTIONAL_TMP_TABLE) continue; lock_type= table->reginfo.lock_type; + DBUG_ASSERT (lock_type != TL_WRITE_DEFAULT); if (lock_type >= TL_WRITE_ALLOW_WRITE) { *write_lock_used=table; diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 1c01248c283..0477c40b42d 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -1537,7 +1537,6 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root, HASH_SEARCH_STATE state; DBUG_ENTER("open_table"); - DBUG_ASSERT (table_list->lock_type != TL_WRITE_DEFAULT); /* find a unused table in the open table cache */ if (refresh) *refresh=0; @@ -2708,11 +2707,6 @@ int open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags) { safe_to_ignore_table= FALSE; - if (tables->lock_type == TL_WRITE_DEFAULT) - { - tables->lock_type= thd->update_lock_default; - DBUG_ASSERT (tables->lock_type >= TL_WRITE_ALLOW_WRITE); - } /* Ignore placeholders for derived tables. After derived tables processing, link to created temporary table will be put here. @@ -2857,7 +2851,8 @@ int open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags) } if (tables->lock_type != TL_UNLOCK && ! thd->locked_tables) - tables->table->reginfo.lock_type=tables->lock_type; + tables->table->reginfo.lock_type= tables->lock_type == TL_WRITE_DEFAULT ? + thd->update_lock_default : tables->lock_type; tables->table->grant= tables->grant; process_view_routines: