Commit 52a37d08 authored by Michael Widenius's avatar Michael Widenius

Fixed bug discovered by testcase for LP#618558 (original bug seams to be fixed):

- Fixed bug in pagecache_delete_internal() when deleting block that was flushed by another thread (fixed bug when block->next_used was unexpectedly null)
Fixed some using uninitialized memory warnings found by valgrind. 

mysql-test/t/information_schema_all_engines-master.opt:
  Added options to make slow test run faster
sql/sp.cc:
  Fixed valgrind warning.
sql/sql_show.cc:
  Fixed valgrind warning.
storage/maria/ma_bitmap.c:
  Fixed wrong call parameter to pagecache_unlock_by_link() that caused assert crash in pagecache
storage/maria/ma_pagecache.c:
  Fixed possible error in pagecache_wait_lock() when hash_link was not set. (We never hit this issue, but could be possible when two threads updates the same page.
  Fixed bug in pagecache_delete_internal() when deleting block that was flushed by another thread (fixed bug when block->next_used was unexpectedly null)
  Cleanup: moved pagecache_pthread_mutex_unlock() over comments and asserts to be just before pagecache_fwrite()
parent 843f2256
--skip-safemalloc --mutex-deadlock-detector=0
......@@ -783,7 +783,7 @@ db_load_routine(THD *thd, int type, sp_name *name, sp_head **sphp,
{
Parser_state parser_state;
if (parser_state.init(thd, defstr.c_ptr(), defstr.length()))
if (parser_state.init(thd, defstr.c_ptr_safe(), defstr.length()))
{
ret= SP_INTERNAL_ERROR;
goto end;
......
......@@ -3944,7 +3944,7 @@ static int get_schema_column_record(THD *thd, TABLE_LIST *tables,
base_type [(dimension)] [unsigned] [zerofill].
For DATA_TYPE column we extract only base type.
*/
tmp_buff= strchr(type.ptr(), '(');
tmp_buff= strchr(type.c_ptr_safe(), '(');
if (!tmp_buff)
/*
if there is no dimention part then check the presence of
......
......@@ -492,7 +492,7 @@ static void _ma_bitmap_unpin_all(MARIA_SHARE *share)
while (pinned_page-- != page_link)
pagecache_unlock_by_link(share->pagecache, pinned_page->link,
pinned_page->unlock, PAGECACHE_UNPIN,
LSN_IMPOSSIBLE, LSN_IMPOSSIBLE, TRUE, TRUE);
LSN_IMPOSSIBLE, LSN_IMPOSSIBLE, FALSE, TRUE);
bitmap->pinned_pages.elements= 0;
DBUG_VOID_RETURN;
}
......
......@@ -2037,12 +2037,12 @@ static PAGECACHE_BLOCK_LINK *find_block(PAGECACHE *pagecache,
KEYCACHE_DBUG_PRINT("find_block", ("block is dirty"));
pagecache_pthread_mutex_unlock(&pagecache->cache_lock);
/*
The call is thread safe because only the current
thread might change the block->hash_link value
*/
DBUG_ASSERT(block->pins == 0);
pagecache_pthread_mutex_unlock(&pagecache->cache_lock);
error= pagecache_fwrite(pagecache,
&block->hash_link->file,
block->buffer,
......@@ -2255,14 +2255,17 @@ static my_bool pagecache_wait_lock(PAGECACHE *pagecache,
#endif
PCBLOCK_INFO(block);
if ((block->status & (PCBLOCK_REASSIGNED | PCBLOCK_IN_SWITCH)) ||
!block->hash_link ||
file.file != block->hash_link->file.file ||
pageno != block->hash_link->pageno)
{
DBUG_PRINT("info", ("the block 0x%lx changed => need retry "
"status: %x files %d != %d or pages %lu != %lu",
(ulong)block, block->status,
file.file, block->hash_link->file.file,
(ulong) pageno, (ulong) block->hash_link->pageno));
file.file,
block->hash_link ? block->hash_link->file.file : -1,
(ulong) pageno,
(ulong) (block->hash_link ? block->hash_link->pageno : 0)));
DBUG_RETURN(1);
}
DBUG_RETURN(0);
......@@ -2611,12 +2614,12 @@ static void read_block(PAGECACHE *pagecache,
*/
pagecache->global_cache_read++;
/* Page is not in buffer yet, is to be read from disk */
pagecache_pthread_mutex_unlock(&pagecache->cache_lock);
/*
Page is not in buffer yet, is to be read from disk
Here other threads may step in and register as secondary readers.
They will register in block->wqueue[COND_FOR_REQUESTED].
*/
pagecache_pthread_mutex_unlock(&pagecache->cache_lock);
error= pagecache_fread(pagecache, &block->hash_link->file,
block->buffer,
block->hash_link->pageno,
......@@ -3450,6 +3453,14 @@ static my_bool pagecache_delete_internal(PAGECACHE *pagecache,
my_bool flush)
{
my_bool error= 0;
if (block->status & PCBLOCK_IN_FLUSH)
{
/*
this call is just 'hint' for the cache to free the page so we will
not interferes with flushing process but gust return success
*/
goto out;
}
if (block->status & PCBLOCK_CHANGED)
{
if (flush)
......@@ -3458,12 +3469,12 @@ static my_bool pagecache_delete_internal(PAGECACHE *pagecache,
KEYCACHE_DBUG_PRINT("find_block", ("block is dirty"));
pagecache_pthread_mutex_unlock(&pagecache->cache_lock);
/*
The call is thread safe because only the current
thread might change the block->hash_link value
*/
DBUG_ASSERT(block->pins == 1);
pagecache_pthread_mutex_unlock(&pagecache->cache_lock);
error= pagecache_fwrite(pagecache,
&block->hash_link->file,
block->buffer,
......@@ -3478,7 +3489,25 @@ static my_bool pagecache_delete_internal(PAGECACHE *pagecache,
block->status|= PCBLOCK_ERROR;
block->error= (int16) my_errno;
my_debug_put_break_here();
goto err;
goto out;
}
}
else
{
PAGECACHE_FILE *filedesc= &block->hash_link->file;
/* We are not going to write the page but have to call callbacks */
DBUG_PRINT("info", ("flush_callback :0x%lx data: 0x%lx"
"write_callback: 0x%lx data: 0x%lx",
(ulong) filedesc->write_callback,
(ulong) filedesc->callback_data));
if ((*filedesc->flush_log_callback)
(block->buffer, block->hash_link->pageno, filedesc->callback_data) ||
(*filedesc->write_callback)
(block->buffer, block->hash_link->pageno, filedesc->callback_data))
{
DBUG_PRINT("error", ("flush or write callback problem"));
error= 1;
goto out;
}
}
pagecache->blocks_changed--;
......@@ -3498,7 +3527,7 @@ static my_bool pagecache_delete_internal(PAGECACHE *pagecache,
/* See NOTE for pagecache_unlock about registering requests. */
free_block(pagecache, block);
err:
out:
dec_counter_for_resize_op(pagecache);
return error;
}
......@@ -4087,6 +4116,7 @@ static void free_block(PAGECACHE *pagecache, PAGECACHE_BLOCK_LINK *block)
("block: %u hash_link 0x%lx",
PCBLOCK_NUMBER(pagecache, block),
(long) block->hash_link));
safe_mutex_assert_owner(&pagecache->cache_lock);
if (block->hash_link)
{
/*
......@@ -4114,6 +4144,8 @@ static void free_block(PAGECACHE *pagecache, PAGECACHE_BLOCK_LINK *block)
KEYCACHE_DBUG_PRINT("free_block",
("block is freed"));
unreg_request(pagecache, block, 0);
DBUG_ASSERT(block->requests == 0);
DBUG_ASSERT(block->next_used != 0);
block->hash_link= NULL;
/* Remove the free block from the LRU ring. */
......@@ -4223,7 +4255,6 @@ static int flush_cached_blocks(PAGECACHE *pagecache,
DBUG_PRINT("info", ("block: %u (0x%lx) to be flushed",
PCBLOCK_NUMBER(pagecache, block), (ulong)block));
PCBLOCK_INFO(block);
pagecache_pthread_mutex_unlock(&pagecache->cache_lock);
DBUG_PRINT("info", ("block: %u (0x%lx) pins: %u",
PCBLOCK_NUMBER(pagecache, block), (ulong)block,
block->pins));
......@@ -4237,6 +4268,7 @@ static int flush_cached_blocks(PAGECACHE *pagecache,
content (see StaleFilePointersInFlush in ma_checkpoint.c).
@todo change argument of functions to be File.
*/
pagecache_pthread_mutex_unlock(&pagecache->cache_lock);
error= pagecache_fwrite(pagecache, &block->hash_link->file,
block->buffer,
block->hash_link->pageno,
......
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