← Back to team overview

maria-developers team mailing list archive

Re: crash in TC_LOG_MMAP::log_one_transaction in maria-10.0.0

 

Kristian Nielsen <knielsen@xxxxxxxxxxxxxxx> writes:

> Sorry, the patch is broken, I didn't test carefully enough.

In fact, I found *another* bug, making it 3 in total...
The first two of these also exists in older MariaDB/MySQL, but may be
hard/impossible to trigger without the third bug, which is something I
introduced in 10.0.

Try this patch, I verified more carefully now that it fixes the problem and
allows your test to run without crashing.

Thanks again for taking the time to report this. Let us know if you find more
issues, and I will look into fixing them as well.

 - Kristian.

=== modified file 'sql/log.cc'
--- sql/log.cc	2012-11-03 11:28:51 +0000
+++ sql/log.cc	2012-11-17 13:35:38 +0000
@@ -7398,8 +7398,9 @@ int TC_LOG_MMAP::open(const char *opt_na
 
   syncing= 0;
   active=pages;
+  DBUG_ASSERT(npages >= 2);
   pool=pages+1;
-  pool_last=pages+npages-1;
+  pool_last_ptr= &((pages+npages-1)->next);
   commit_ordered_queue= NULL;
   commit_ordered_queue_busy= false;
 
@@ -7432,8 +7433,8 @@ void TC_LOG_MMAP::get_active_from_pool()
   do
   {
     best_p= p= &pool;
-    if ((*p)->waiters == 0) // can the first page be used ?
-      break;                // yes - take it.
+    if ((*p)->waiters == 0 && (*p)->free > 0) // can the first page be used ?
+      break;                                  // yes - take it.
 
     best_free=0;            // no - trying second strategy
     for (p=&(*p)->next; *p; p=&(*p)->next)
@@ -7450,10 +7451,10 @@ void TC_LOG_MMAP::get_active_from_pool()
   mysql_mutex_assert_owner(&LOCK_active);
   active=*best_p;
 
-  if ((*best_p)->next)              // unlink the page from the pool
-    *best_p=(*best_p)->next;
-  else
-    pool_last=*best_p;
+  /* Unlink the page from the pool. */
+  if (!(*best_p)->next)
+    pool_last_ptr= best_p;
+  *best_p=(*best_p)->next;
   mysql_mutex_unlock(&LOCK_pool);
 
   mysql_mutex_lock(&active->lock);
@@ -7617,8 +7618,8 @@ int TC_LOG_MMAP::sync()
 
   /* page is synced. let's move it to the pool */
   mysql_mutex_lock(&LOCK_pool);
-  pool_last->next=syncing;
-  pool_last=syncing;
+  (*pool_last_ptr)=syncing;
+  pool_last_ptr=&(syncing->next);
   syncing->next=0;
   syncing->state= err ? PS_ERROR : PS_POOL;
   mysql_cond_signal(&COND_pool);           // in case somebody's waiting

=== modified file 'sql/log.h'
--- sql/log.h	2012-09-30 23:30:44 +0000
+++ sql/log.h	2012-11-18 11:09:27 +0000
@@ -118,7 +118,7 @@ class TC_LOG_MMAP: public TC_LOG
   struct pending_cookies {
     uint count;
     uint pending_count;
-    ulong cookies[TC_LOG_PAGE_SIZE];
+    ulong cookies[TC_LOG_PAGE_SIZE/sizeof(my_xid)];
   };
 
   private:
@@ -145,7 +145,7 @@ class TC_LOG_MMAP: public TC_LOG
   my_off_t file_length;
   uint npages, inited;
   uchar *data;
-  struct st_page *pages, *syncing, *active, *pool, *pool_last;
+  struct st_page *pages, *syncing, *active, *pool, **pool_last_ptr;
   /*
     note that, e.g. LOCK_active is only used to protect
     'active' pointer, to protect the content of the active page


Follow ups

References