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).
parent a486f955
......@@ -874,6 +874,8 @@ end:
int lock_table_name(THD *thd, TABLE_LIST *table_list, bool check_in_use)
{
TABLE *table;
TABLE_SHARE *share;
char *key_buff;
char key[MAX_DBKEY_LENGTH];
char *db= table_list->db;
uint key_length;
......@@ -903,17 +905,18 @@ int lock_table_name(THD *thd, TABLE_LIST *table_list, bool check_in_use)
}
/*
Create a table entry with the right key and with an old refresh version
Note that we must use my_malloc() here as this is freed by the table
cache
Note that we must use my_multi_malloc() here as this is freed by the
table cache
*/
if (!(table= (TABLE*) my_malloc(sizeof(*table)+ sizeof(TABLE_SHARE)+
key_length, MYF(MY_WME | MY_ZEROFILL))))
if (!my_multi_malloc(MYF(MY_WME | MY_ZEROFILL),
&table, sizeof(*table),
&share, sizeof(*share),
&key_buff, key_length,
NULL))
DBUG_RETURN(-1);
table->s= (TABLE_SHARE*) (table+1);
memcpy((table->s->table_cache_key.str= (char*) (table->s+1)), key,
key_length);
table->s->table_cache_key.length= key_length;
table->s->tmp_table= INTERNAL_TMP_TABLE; // for intern_close_table
table->s= share;
share->set_table_cache_key(key_buff, key, key_length);
share->tmp_table= INTERNAL_TMP_TABLE; // for intern_close_table
table->in_use= thd;
table->locked_by_name=1;
table_list->table=table;
......
......@@ -634,6 +634,7 @@ TABLE_SHARE *get_cached_table_share(const char *db, const char *table_name)
static void close_handle_and_leave_table_as_lock(TABLE *table)
{
TABLE_SHARE *share, *old_share= table->s;
char *key_buff;
MEM_ROOT *mem_root= &table->mem_root;
DBUG_ENTER("close_handle_and_leave_table_as_lock");
......@@ -642,20 +643,14 @@ static void close_handle_and_leave_table_as_lock(TABLE *table)
This has to be done to ensure that the table share is removed from
the table defintion cache as soon as the last instance is removed
*/
if ((share= (TABLE_SHARE*) alloc_root(mem_root, sizeof(*share))))
if (multi_alloc_root(mem_root,
&share, sizeof(*share),
&key_buff, old_share->table_cache_key.length,
NULL))
{
bzero((char*) share, sizeof(*share));
share->db.str= memdup_root(mem_root, old_share->db.str,
old_share->db.length+1);
share->db.length= old_share->db.length;
share->table_name.str= memdup_root(mem_root,
old_share->table_name.str,
old_share->table_name.length+1);
share->table_name.length= old_share->table_name.length;
share->table_cache_key.str= memdup_root(mem_root,
old_share->table_cache_key.str,
old_share->table_cache_key.length);
share->table_cache_key.length= old_share->table_cache_key.length;
share->set_table_cache_key(key_buff, old_share->table_cache_key.str,
old_share->table_cache_key.length);
share->tmp_table= INTERNAL_TMP_TABLE; // for intern_close_table()
}
......@@ -1603,28 +1598,18 @@ bool rename_temporary_table(THD* thd, TABLE *table, const char *db,
const char *table_name)
{
char *key;
uint key_length;
TABLE_SHARE *share= table->s;
TABLE_LIST table_list;
uint db_length, table_length;
DBUG_ENTER("rename_temporary_table");
if (!(key=(char*) alloc_root(&share->mem_root,
(uint) (db_length= strlen(db))+
(uint) (table_length= strlen(table_name))+6+4)))
if (!(key=(char*) alloc_root(&share->mem_root, MAX_DBKEY_LENGTH)))
DBUG_RETURN(1); /* purecov: inspected */
table_list.db= (char*) db;
table_list.table_name= (char*) table_name;
share->db.str= share->table_cache_key.str= key;
share->db.length= db_length;
share->table_cache_key.length= create_table_def_key(thd, key,
&table_list, 1);
/*
Here we use the fact that table_name is stored as the second component
in the 'key' (after db_name), where components are separated with \0
*/
share->table_name.str= key+db_length+1;
share->table_name.length= table_length;
key_length= create_table_def_key(thd, key, &table_list, 1);
share->set_table_cache_key(key, key_length);
DBUG_RETURN(0);
}
......@@ -1749,10 +1734,7 @@ bool reopen_name_locked_table(THD* thd, TABLE_LIST* table_list)
{
TABLE *table= table_list->table;
TABLE_SHARE *share;
char *db= table_list->db;
char *table_name= table_list->table_name;
char key[MAX_DBKEY_LENGTH];
uint key_length;
TABLE orig_table;
DBUG_ENTER("reopen_name_locked_table");
......@@ -1762,7 +1744,6 @@ bool reopen_name_locked_table(THD* thd, TABLE_LIST* table_list)
DBUG_RETURN(TRUE);
orig_table= *table;
key_length=(uint) (strmov(strmov(key,db)+1,table_name)-key)+1;
if (open_unireg_entry(thd, table, table_list, table_name,
table->s->table_cache_key.str,
......
......@@ -8645,8 +8645,6 @@ create_tmp_table(THD *thd,TMP_TABLE_PARAM *param,List<Item> &fields,
share->primary_key= MAX_KEY; // Indicate no primary key
share->keys_for_keyread.init();
share->keys_in_use.init();
/* For easier error reporting */
share->table_cache_key= share->db;
/* Calculate which type of fields we will store in the temporary table */
......
......@@ -93,6 +93,7 @@ TABLE_SHARE *alloc_table_share(TABLE_LIST *table_list, char *key,
{
MEM_ROOT mem_root;
TABLE_SHARE *share;
char *key_buff, *path_buff;
char path[FN_REFLEN];
uint path_length;
DBUG_ENTER("alloc_table_share");
......@@ -103,22 +104,17 @@ TABLE_SHARE *alloc_table_share(TABLE_LIST *table_list, char *key,
table_list->db,
table_list->table_name, "", 0);
init_sql_alloc(&mem_root, TABLE_ALLOC_BLOCK_SIZE, 0);
if ((share= (TABLE_SHARE*) alloc_root(&mem_root,
sizeof(*share) + key_length +
path_length +1)))
if (multi_alloc_root(&mem_root,
&share, sizeof(*share),
&key_buff, key_length,
&path_buff, path_length + 1,
NULL))
{
bzero((char*) share, sizeof(*share));
share->table_cache_key.str= (char*) (share+1);
share->table_cache_key.length= key_length;
memcpy(share->table_cache_key.str, key, key_length);
/* Use the fact the key is db/0/table_name/0 */
share->db.str= share->table_cache_key.str;
share->db.length= strlen(share->db.str);
share->table_name.str= share->db.str + share->db.length + 1;
share->table_name.length= strlen(share->table_name.str);
share->set_table_cache_key(key_buff, key, key_length);
share->path.str= share->table_cache_key.str+ key_length;
share->path.str= path_buff;
share->path.length= path_length;
strmov(share->path.str, path);
share->normalized_path.str= share->path.str;
......
......@@ -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;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment