[Commits] Rev 2971: Fixed bug discovered by testcase for LP#618558 (original bug seams to be fixed): in lp:maria/5.1

Michael Widenius monty at askmonty.org
Mon Nov 15 22:44:42 EET 2010


At lp:maria/5.1

------------------------------------------------------------
revno: 2971
revision-id: monty at askmonty.org-20101115204441-dv0b6txumsm0g1w1
parent: monty at askmonty.org-20101107122529-sl5sup1m9gjxlzte
committer: Michael Widenius <monty at askmonty.org>
branch nick: maria-5.1
timestamp: Mon 2010-11-15 22:44:41 +0200
message:
  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. 
-------------- next part --------------
=== added file 'mysql-test/t/information_schema_all_engines-master.opt'
--- a/mysql-test/t/information_schema_all_engines-master.opt	1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/information_schema_all_engines-master.opt	2010-11-15 20:44:41 +0000
@@ -0,0 +1 @@
+--skip-safemalloc --mutex-deadlock-detector=0

=== modified file 'sql/sp.cc'
--- a/sql/sp.cc	2010-09-03 16:20:30 +0000
+++ b/sql/sp.cc	2010-11-15 20:44:41 +0000
@@ -783,7 +783,7 @@ db_load_routine(THD *thd, int type, sp_n
 
   {
     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;

=== modified file 'sql/sql_show.cc'
--- a/sql/sql_show.cc	2010-10-19 13:58:35 +0000
+++ b/sql/sql_show.cc	2010-11-15 20:44:41 +0000
@@ -3944,7 +3944,7 @@ static int get_schema_column_record(THD
       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

=== modified file 'storage/maria/ma_bitmap.c'
--- a/storage/maria/ma_bitmap.c	2010-11-03 12:14:02 +0000
+++ b/storage/maria/ma_bitmap.c	2010-11-15 20:44:41 +0000
@@ -492,7 +492,7 @@ static void _ma_bitmap_unpin_all(MARIA_S
   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;
 }

=== modified file 'storage/maria/ma_pagecache.c'
--- a/storage/maria/ma_pagecache.c	2010-06-15 21:39:28 +0000
+++ b/storage/maria/ma_pagecache.c	2010-11-15 20:44:41 +0000
@@ -2037,12 +2037,12 @@ static PAGECACHE_BLOCK_LINK *find_block(
 
             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(PAGEC
 #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 *pageca
     */
 
     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
                                          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
 
       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
         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
   /* 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 *pageca
                       ("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 *pageca
   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
     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
       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,



More information about the commits mailing list