From 992aceaf71fec15cef86d31550e1f7428bbbb8e9 Mon Sep 17 00:00:00 2001 From: unknown Date: Mon, 21 Aug 2006 19:02:11 +0400 Subject: Fix for bug#19403/12212 "Crash that happens during removing of database name from cache" and #21216 "Simultaneous DROP TABLE and SHOW OPEN TABLES causes server to crash". Crash happened when one ran DROP DATABASE or SHOW OPEN TABLES statements while concurrently doing DROP TABLE (or RENAME TABLE, CREATE TABLE LIKE or any other command that takes name-lock) in other connection. This problem was caused by the fact that table placeholders which were added to table cache in order to obtain name-lock on table had TABLE_SHARE::db and table_name set to 0. Therefore they broke assumption that these members are non-0 for all tables in table cache on which some of our code relies. The fix sets these members for such placeholders to appropriate value making this assumption true again. As attempt to avoid such problems in future we introduce auxiliary TABLE_SHARE::set_table_cache_key() methods which should be used when one wants to set TABLE_SHARE::table_cache_key and which ensure that TABLE_SHARE::table_name/db are set properly. Test cases for these bugs were added to 5.0 test-suite (with 5.0-specific fix for bug #21216). sql/lock.cc: Our code assumes that TABLE_SHARE::table_name/db for objects in table cache is set properly (and is non-NULL). For example look in list_open_tables() and remove_db_from_cache(). This was not true for table placeholders that were added to table cache for name-locking. Changed lock_table_name() to preserve this assumption (now it uses TABLE_SHARE::set_table_cache_key() to set all three table_cache_key/db/table_name members at once). Also now we use my_multi_malloc() to allocate memory for TABLE and TABLE_SHARE objects instead of using my_malloc() + summing sizes as it automatically provides proper alignment for memory allocated. sql/sql_base.cc: Now we use TABLE_SHARE::set_table_cache_key() auxiliary methods to set TABLE_SHARE::table_cache_key/db/table_name members at once. We also use multi_alloc_root() instead of alloc_root() for allocating memory for several objects as it is less error prone. Finally, we also got rid of unused code in reopen_name_locked_table(). sql/sql_select.cc: Got rid of redundant code. TABLE_SHARE::db/table_cache_key are both set to empty string by the call to init_tmp_table_share() routine. sql/table.cc: Now alloc_table_share() uses auxiliary TABLE_SHARE::set_table_cache_key() method to properly set TABLE_SHARE::table_cache_key/db/table_name members. Also now we use multi_alloc_root() instead of alloc_root() for allocating memory for several objects as it is more clear/less error-prone. sql/table.h: Added comment about importance of apropriate setting of TABLE_SHARE::table_name/db/table_cache_key for tables in table cache. Introduced two auxiliary TABLE_SHARE::set_table_cache_key() methods which allow to set these three members at once. --- sql/table.h | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) (limited to 'sql/table.h') diff --git a/sql/table.h b/sql/table.h index 7675c27823b..c13a23468ae 100644 --- a/sql/table.h +++ b/sql/table.h @@ -138,7 +138,16 @@ typedef struct st_table_share CHARSET_INFO *table_charset; /* Default charset of string fields */ MY_BITMAP all_set; - /* A pair "database_name\0table_name\0", widely used as simply a db name */ + /* + Key which is used for looking-up table in table cache and in the list + of thread's temporary tables. Has the form of: + "database_name\0table_name\0" + optional part for temporary tables. + + Note that all three 'table_cache_key', 'db' and 'table_name' members + must be set (and be non-zero) for tables in table cache. They also + should correspond to each other. + To ensure this one can use set_table_cache() methods. + */ LEX_STRING table_cache_key; LEX_STRING db; /* Pointer to db */ LEX_STRING table_name; /* Table name (for open) */ @@ -223,6 +232,60 @@ typedef struct st_table_share uint part_state_len; handlerton *default_part_db_type; #endif + + + /* + Set share's table cache key and update its db and table name appropriately. + + SYNOPSIS + set_table_cache_key() + key_buff Buffer with already built table cache key to be + referenced from share. + key_length Key length. + + NOTES + Since 'key_buff' buffer will be referenced from share it should has same + life-time as share itself. + This method automatically ensures that TABLE_SHARE::table_name/db have + appropriate values by using table cache key as their source. + */ + + void set_table_cache_key(char *key_buff, uint key_length) + { + table_cache_key.str= key_buff; + table_cache_key.length= key_length; + /* + Let us use the fact that the key is "db/0/table_name/0" + optional + part for temporary tables. + */ + db.str= table_cache_key.str; + db.length= strlen(db.str); + table_name.str= db.str + db.length + 1; + table_name.length= strlen(table_name.str); + } + + + /* + Set share's table cache key and update its db and table name appropriately. + + SYNOPSIS + set_table_cache_key() + key_buff Buffer to be used as storage for table cache key + (should be at least key_length bytes). + key Value for table cache key. + key_length Key length. + + NOTE + Since 'key_buff' buffer will be used as storage for table cache key + it should has same life-time as share itself. + */ + + void set_table_cache_key(char *key_buff, const char *key, uint key_length) + { + memcpy(key_buff, key, key_length); + set_table_cache_key(key_buff, key_length); + } + } TABLE_SHARE; -- cgit v1.2.1