Commit b4dd13b5 authored by Sergei Golubchik's avatar Sergei Golubchik

MDEV-5405 RQG induced crash in mi_assign_to_key_cache in safe mutex unlock

if two threads were calling mi_assign_to_key_cache() for the same table,
one could change share->key_cache while the other was having
share->key_cache->op_lock locked. The other thread would crash then,
trying to unlock share->key_cache->op_lock (because it would be a different mutex).

fixed by caching the value of share->key_cache in a local variable. The thread can still
call flush_key_blocks() for an unassigned keycache, but it's harmless.
parent ebaac51c
create table t1 (f int, key(f)) engine=myisam;
set global kc1.key_buffer_size = 65536;
set debug_sync='assign_key_cache_op_unlock wait_for op_locked';
cache index t1 in kc1;
set debug_sync='assign_key_cache_op_lock signal op_locked wait_for assigned';
cache index t1 in kc1;
Table Op Msg_type Msg_text
test.t1 assign_to_keycache status OK
set debug_sync='now signal assigned';
Table Op Msg_type Msg_text
test.t1 assign_to_keycache status OK
drop table t1;
set global kc1.key_buffer_size = 0;
set debug_sync='reset';
#
# MDEV-5405 RQG induced crash in mi_assign_to_key_cache in safe mutex unlock
#
--source include/have_debug_sync.inc
create table t1 (f int, key(f)) engine=myisam;
set global kc1.key_buffer_size = 65536;
connect (con1, localhost, root);
set debug_sync='assign_key_cache_op_unlock wait_for op_locked';
send cache index t1 in kc1;
connection default;
sleep 1;
set debug_sync='assign_key_cache_op_lock signal op_locked wait_for assigned';
send cache index t1 in kc1;
connection con1;
reap;
set debug_sync='now signal assigned';
disconnect con1;
connection default;
reap;
drop table t1;
set global kc1.key_buffer_size = 0;
set debug_sync='reset';
...@@ -49,19 +49,20 @@ ...@@ -49,19 +49,20 @@
int mi_assign_to_key_cache(MI_INFO *info, int mi_assign_to_key_cache(MI_INFO *info,
ulonglong key_map __attribute__((unused)), ulonglong key_map __attribute__((unused)),
KEY_CACHE *key_cache) KEY_CACHE *new_key_cache)
{ {
int error= 0; int error= 0;
MYISAM_SHARE* share= info->s; MYISAM_SHARE* share= info->s;
KEY_CACHE *old_key_cache= share->key_cache;
DBUG_ENTER("mi_assign_to_key_cache"); DBUG_ENTER("mi_assign_to_key_cache");
DBUG_PRINT("enter",("old_key_cache_handle: 0x%lx new_key_cache_handle: 0x%lx", DBUG_PRINT("enter",("old_key_cache_handle: %p new_key_cache_handle: %p",
(long) share->key_cache, (long) key_cache)); old_key_cache, new_key_cache));
/* /*
Skip operation if we didn't change key cache. This can happen if we Skip operation if we didn't change key cache. This can happen if we
call this for all open instances of the same table call this for all open instances of the same table
*/ */
if (share->key_cache == key_cache) if (old_key_cache == new_key_cache)
DBUG_RETURN(0); DBUG_RETURN(0);
/* /*
...@@ -76,15 +77,17 @@ int mi_assign_to_key_cache(MI_INFO *info, ...@@ -76,15 +77,17 @@ int mi_assign_to_key_cache(MI_INFO *info,
in the old key cache. in the old key cache.
*/ */
pthread_mutex_lock(&share->key_cache->op_lock); pthread_mutex_lock(&old_key_cache->op_lock);
if (flush_key_blocks(share->key_cache, share->kfile, &share->dirty_part_map, DEBUG_SYNC_C("assign_key_cache_op_lock");
if (flush_key_blocks(old_key_cache, share->kfile, &share->dirty_part_map,
FLUSH_RELEASE)) FLUSH_RELEASE))
{ {
error= my_errno; error= my_errno;
mi_print_error(info->s, HA_ERR_CRASHED); mi_print_error(info->s, HA_ERR_CRASHED);
mi_mark_crashed(info); /* Mark that table must be checked */ mi_mark_crashed(info); /* Mark that table must be checked */
} }
pthread_mutex_unlock(&share->key_cache->op_lock); pthread_mutex_unlock(&old_key_cache->op_lock);
DEBUG_SYNC_C("assign_key_cache_op_unlock");
/* /*
Flush the new key cache for this file. This is needed to ensure Flush the new key cache for this file. This is needed to ensure
...@@ -94,7 +97,7 @@ int mi_assign_to_key_cache(MI_INFO *info, ...@@ -94,7 +97,7 @@ int mi_assign_to_key_cache(MI_INFO *info,
(This can never fail as there is never any not written data in the (This can never fail as there is never any not written data in the
new key cache) new key cache)
*/ */
(void) flush_key_blocks(key_cache, share->kfile, &share->dirty_part_map, (void) flush_key_blocks(new_key_cache, share->kfile, &share->dirty_part_map,
FLUSH_RELEASE); FLUSH_RELEASE);
/* /*
...@@ -106,13 +109,13 @@ int mi_assign_to_key_cache(MI_INFO *info, ...@@ -106,13 +109,13 @@ int mi_assign_to_key_cache(MI_INFO *info,
Tell all threads to use the new key cache Tell all threads to use the new key cache
This should be seen at the lastes for the next call to an myisam function. This should be seen at the lastes for the next call to an myisam function.
*/ */
share->key_cache= key_cache; share->key_cache= new_key_cache;
share->dirty_part_map= 0; share->dirty_part_map= 0;
/* store the key cache in the global hash structure for future opens */ /* store the key cache in the global hash structure for future opens */
if (multi_key_cache_set((uchar*) share->unique_file_name, if (multi_key_cache_set((uchar*) share->unique_file_name,
share->unique_name_length, share->unique_name_length,
share->key_cache)) new_key_cache))
error= my_errno; error= my_errno;
mysql_mutex_unlock(&share->intern_lock); mysql_mutex_unlock(&share->intern_lock);
DBUG_RETURN(error); DBUG_RETURN(error);
......
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