Fix for: LP #634955: Assert in _ma_update_at_original_place()

Added locking of lock mutex when updating status in external_unlock() for Aria and MyISAM tables.
Fixed that 'source' command doesn't cause mysql command line tool to exit on error.
DEBUG_EXECUTE() and DEBUG_EVALUATE_IF() should not execute things based on wildcards. (Allows one to run --debug with mysql-test-run scripts that uses @debug)
Fixed several core dump, deadlock and crashed table bugs in handling of LOCK TABLE with MERGE tables:
- Added priority of locks to avoid crashes with MERGE tables.
- Added thr_lock_merge() to allow one to merge two results of thr_lock().
Fixed 'not found row' bug in REPLACE with Aria tables.
Mark MyISAM tables that are part of MERGE with HA_OPEN_MERGE_TABLE and set the locks to have priority THR_LOCK_MERGE_PRIV.
- By sorting MERGE tables last in thr_multi_unlock() it's safer to release and relock them many times (can happen when TRIGGERS are created)
Avoid printing (null) in debug file (to easier find out wrong NULL pointer usage with %s).



client/mysql.cc:
  Fixed that 'source' command doesn't cause mysql command line tool to exit on error.
client/mysqltest.cc:
  Don't send NULL to fn_format(). (Can cause crash on Solaris when using --debug)
dbug/dbug.c:
  DEBUG_EXECUTE() and DEBUG_EVALUATE_IF() should not execute things based on wildcards.
include/my_base.h:
  Added flag to signal if one opens a MERGE table.
  Added extra() command to signal that one is not part of a MERGE table anymore.
include/thr_lock.h:
  Added priority for locks (needed to fix bug in thr_lock when using MERGE tables)
  Added option to thr_unlock() if get_status() should be called.
  Added prototype for thr_merge_locks().
mysql-test/mysql-test-run.pl:
  Ignore crashed table warnings for tables named 'crashed'.
mysql-test/r/merge.result:
  Renamed triggers to make debugging easier.
  Added some CHECK TABLES to catch errors earlier.
  Additional tests.
mysql-test/r/merge_debug.result:
  Test of error handling when reopening MERGE tables.
mysql-test/r/udf_query_cache.result:
  Added missing flush status
mysql-test/suite/parts/r/partition_repair_myisam.result:
  Update results
mysql-test/t/merge.test:
  Renamed triggers to make debugging easier.
  Added some CHECK TABLES to catch errors earlier.
  Additional tests.
mysql-test/t/merge_debug.test:
  Test of error handling when reopening MERGE tables.
mysql-test/t/udf_query_cache.test:
  Added missing flush status
mysys/my_getopt.c:
  Removed not used variable
mysys/my_symlink2.c:
  Changed (null) to (NULL) to make it easier to find NULL arguments to DBUG_PRINT() functions.
  (On linux, NULL to sprintf is printed 'null')
mysys/thr_lock.c:
  Added priority of locks to avoid crashes with MERGE tables.
  Added thr_lock_merge() to allow one to merge two results of thr_lock().
  - This is needed for MyISAM as all locked table must share the same status. If not, you will not see newly inserted rows in other instances of the table.
  If calling thr_unlock() with THR_UNLOCK_UPDATE_STATUS, call update_status() and restore_status() for the locks. This is needed in some rare cases where we call thr_unlock() followed by thr_lock() without calling external_unlock/external_lock in between.
  Simplify loop in thr_multi_lock().
  Added 'start_trans', which is called at end of thr_multi_lock() when all locks are taken.
  - This was needed by Aria to ensure that transaction is started when we got all locks, not at get_status(). Without this, some rows could not be visible when we lock two tables at the same time, causing REPLACE using two tables to fail unexpectedly.
sql/handler.cc:
  Add an assert() in handler::print_error() for "impossible errors" (like table is crashed) when --debug-assert-if-crashed-table is used.
sql/lock.cc:
  Simplify mysql_lock_tables() code if get_lock_data() returns 0 locks.
  Added new parameter to thr_multi_unlock()
  In mysql_unlock_read_tables(), call first externa_unlock(), then thr_multi_unlock();  This is same order as we do in mysql_unlock_tables().
  Don't abort locks in mysql_lock_abort() for merged tables when a MERGE table is deleted; Would cause a spin lock.
  Added call to thr_merge_locks() in mysql_lock_merge() to ensure consistency in thr_locks().
  - New locks of same type and table is stored after the old lock to ensure that we get the status from the original lock.
sql/mysql_priv.h:
  Added debug_assert_if_crashed_table
sql/mysqld.cc:
  Added --debug-assert-if-crashed-table
sql/parse_file.cc:
  Don't print '(null)' in DBUG_PRINT of no dir given
sql/set_var.cc:
  Increase default size of buffer for @debug variable.
sql/sql_base.cc:
  In case of error from reopen_table() in reopen_tables(), call unlock_open_table() and restart loop.
  - This fixed bug when we twice deleted same table from open_cache.
  Don't take name lock for already name locked table in open_unireg_entry().
  - Fixed bug when doing repair in reopen_table().
  - In detach_merge_children(), always detach if 'clear_refs' is given. We can't trust parent->children_attached as this function can be called twice, first time with clear_refs set to 0.
sql/sql_class.cc:
  Changed printing of (null) to "" in set_thd_proc_info()
sql/sql_parse.cc:
  Added DBUG
sql/sql_trigger.cc:
  Don't call unlink_open_table() if reopen_table() fails as the table may already be freed.
storage/maria/ma_bitmap.c:
  Fixed DBUG_ASSERT() in allocate_tail()
storage/maria/ma_blockrec.c:
  Fixed wrong calculation of row length for very small rows in undo_row_update().
  - Fixes ASSERT() when doing undo.
storage/maria/ma_blockrec.h:
  Added _ma_block_start_trans() and _ma_block_start_trans_no_versioning()
storage/maria/ma_locking.c:
  Call _ma_update_status_with_lock() when releasing write locks.
  - Fixes potential problem with updating status without the proper lock.
storage/maria/ma_open.c:
  Changed to use start_trans() instead of get_status() to ensure that we see all rows in all locked tables when we got the locks.
  - Fixed 'not found row' bug in REPLACE with Aria tables.
storage/maria/ma_state.c:
  Added _ma_update_status_with_lock() and _ma_block_start_trans().
  This is to ensure that we see all rows in all locked tables when we got the locks.
storage/maria/ma_state.h:
  Added _ma_update_status_with_lock()
storage/maria/ma_write.c:
  More DBUG_PRINT
storage/myisam/mi_check.c:
  Fixed error message
storage/myisam/mi_extra.c:
  Added HA_EXTRA_DETACH_CHILD:
  - Detach MyISAM table to not be part of MERGE table (remove flag & lock priority).
storage/myisam/mi_locking.c:
  Call mi_update_status_with_lock() when releasing write locks.
  - Fixes potential problem with updating status without the proper lock.
  Change to use new HA_OPEN_MERGE_TABLE flag to test if MERGE table.
  Added mi_fix_status(), called by thr_merge().
storage/myisam/mi_open.c:
  Added marker if part of MERGE table.
  Call mi_fix_status() in thr_lock() for transactional tables.
storage/myisam/myisamdef.h:
  Change my_once_flag to uint, as it stored different values than just 0/1
  Added 'open_flag' to store state given to mi_open()
storage/myisammrg/ha_myisammrg.cc:
  Add THR_LOCK_MERGE_PRIV to THR_LOCK_DATA to get MERGE locks sorted after other types of locks.
storage/myisammrg/myrg_locking.c:
  Remove windows specific code.
storage/myisammrg/myrg_open.c:
  Use HA_OPEN_MERGE_TABLE to mi_open().
  Set HA_OPEN_MERGE_TABLE for linked MyISAM tables.
storage/xtradb/buf/buf0buf.c:
  Fixed compiler warning
storage/xtradb/buf/buf0lru.c:
  Initialize variable that could be used not initialized.

3 files changed, 151 insertions, 61 deletions

diff --git a/mysys/my_getopt.c b/mysys/my_getopt.c
index f9a2c17ae7c..7281f2e1420 100644
--- a/mysys/my_getopt.c
+++ b/mysys/my_getopt.c
@@ -614,7 +614,6 @@ static int setval(const struct my_option *opts, void *value, char *argument,
 		  my_bool set_maximum_value)
 {
   int err= 0;
-  int pos;
 
   if (value && argument)
   {
diff --git a/mysys/my_symlink2.c b/mysys/my_symlink2.c
index 7c3ddbb911c..bc7ac751fad 100644
--- a/mysys/my_symlink2.c
+++ b/mysys/my_symlink2.c
@@ -34,8 +34,8 @@ File my_create_with_symlink(const char *linkname, const char *filename,
   char abs_linkname[FN_REFLEN];
   DBUG_ENTER("my_create_with_symlink");
   DBUG_PRINT("enter", ("linkname: %s  filename: %s",
-                       linkname ? linkname : "(null)",
-                       filename ? filename : "(null)"));
+                       linkname ? linkname : "(NULL)",
+                       filename ? filename : "(NULL)"));
 
   if (my_disable_symlinks)
   {
diff --git a/mysys/thr_lock.c b/mysys/thr_lock.c
index b8aa9e5fcc0..9c8236ae0e6 100644
--- a/mysys/thr_lock.c
+++ b/mysys/thr_lock.c
@@ -63,6 +63,11 @@ update_status:
         A storage engine should also call update_status internally
         in the ::external_lock(F_UNLCK) method.
         In MyISAM and CSV this functions updates the length of the datafile.
+        MySQL does in some exceptional cases (when doing DLL statements on
+        open tables calls thr_unlock() followed by thr_lock() without calling
+        ::external_lock() in between. In this case thr_unlock() is called with
+        the THR_UNLOCK_UPDATE_STATUS flag and thr_unlock() will call
+        update_status for write locks.
 get_status:
 	When one gets a lock this functions is called.
 	In MyISAM this stores the number of rows and size of the datafile
@@ -105,8 +110,30 @@ static inline pthread_cond_t *get_cond(void)
   return &my_thread_var->suspend;
 }
 
+
+/*
+  Priority for locks (decides in which order locks are locked)
+  We want all write locks to be first, followed by read locks.
+  Locks from MERGE tables has a little lower priority than other
+  locks, to allow one to release merge tables without having
+  to unlock and re-lock other locks.
+  The lower the number, the higher the priority for the lock.
+  Read locks should have 4, write locks should have 0.
+  UNLOCK is 8, to force these last in thr_merge_locks.
+  For MERGE tables we add 2 (THR_LOCK_MERGE_PRIV) to the lock priority.
+  THR_LOCK_LATE_PRIV (1) is used when one locks other tables to be merged
+  with existing locks. This way we prioritize the original locks over the
+  new locks.
+*/
+
+static uint lock_priority[(uint)TL_WRITE_ONLY+1] =
+{ 8, 4, 4, 4, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0};
+
+#define LOCK_CMP(A,B) ((uchar*) ((A)->lock) + lock_priority[(uint) (A)->type] + (A)->priority < (uchar*) ((B)->lock) + lock_priority[(uint) (B)->type] + (B)->priority)
+
+
 /*
-** For the future (now the thread specific cond is alloced by my_pthread.c)
+  For the future (now the thread specific cond is alloced by my_pthread.c)
 */
 
 my_bool init_thr_lock()
@@ -530,7 +557,7 @@ wait_for_lock(struct st_lock_list *wait, THR_LOCK_DATA *data,
 }
 
 
-enum enum_thr_lock_result
+static enum enum_thr_lock_result
 thr_lock(THR_LOCK_DATA *data, THR_LOCK_OWNER *owner,
          enum thr_lock_type lock_type)
 {
@@ -544,6 +571,7 @@ thr_lock(THR_LOCK_DATA *data, THR_LOCK_OWNER *owner,
   data->cond=0;					/* safety */
   data->type=lock_type;
   data->owner= owner;                           /* Must be reset ! */
+  data->priority&= ~THR_LOCK_LATE_PRIV;
   VOID(pthread_mutex_lock(&lock->mutex));
   DBUG_PRINT("lock",("data: 0x%lx  thread: 0x%lx  lock: 0x%lx  type: %d",
                      (long) data, data->owner->info->thread_id,
@@ -808,7 +836,7 @@ static inline void free_all_read_locks(THR_LOCK *lock,
 
 	/* Unlock lock and free next thread on same lock */
 
-void thr_unlock(THR_LOCK_DATA *data)
+void thr_unlock(THR_LOCK_DATA *data, uint unlock_flags)
 {
   THR_LOCK *lock=data->lock;
   enum thr_lock_type lock_type=data->type;
@@ -832,6 +860,21 @@ void thr_unlock(THR_LOCK_DATA *data)
   }
   else
     lock->write.last=data->prev;
+ 
+  if (unlock_flags & THR_UNLOCK_UPDATE_STATUS)
+  {
+    /* External lock was not called; Update or restore status */
+    if (lock_type >= TL_WRITE_CONCURRENT_INSERT)
+    {
+      if (lock->update_status)
+        (*lock->update_status)(data->status_param);
+    }
+    else
+    {
+      if (lock->restore_status)
+        (*lock->restore_status)(data->status_param);
+    }
+  }
   if (lock_type == TL_READ_NO_INSERT)
     lock->read_no_write_count--;
   data->type=TL_UNLOCK;				/* Mark unlocked */
@@ -967,14 +1010,12 @@ end:
 
 
 /*
-** Get all locks in a specific order to avoid dead-locks
-** Sort acording to lock position and put write_locks before read_locks if
-** lock on same lock.
+  Get all locks in a specific order to avoid dead-locks
+  Sort acording to lock position and put write_locks before read_locks if
+  lock on same lock. Locks on MERGE tables has lower priority than other
+  locks of the same type. See comment for lock_priority.
 */
 
-
-#define LOCK_CMP(A,B) ((uchar*) (A->lock) - (uint) ((A)->type) < (uchar*) (B->lock)- (uint) ((B)->type))
-
 static void sort_locks(THR_LOCK_DATA **data,uint count)
 {
   THR_LOCK_DATA **pos,**end,**prev,*tmp;
@@ -999,18 +1040,22 @@ static void sort_locks(THR_LOCK_DATA **data,uint count)
 enum enum_thr_lock_result
 thr_multi_lock(THR_LOCK_DATA **data, uint count, THR_LOCK_OWNER *owner)
 {
-  THR_LOCK_DATA **pos,**end;
+  THR_LOCK_DATA **pos, **end, **first_lock;
   DBUG_ENTER("thr_multi_lock");
   DBUG_PRINT("lock",("data: 0x%lx  count: %d", (long) data, count));
+
   if (count > 1)
     sort_locks(data,count);
+  else if (count == 0)
+    DBUG_RETURN(THR_LOCK_SUCCESS);
+
   /* lock everything */
   for (pos=data,end=data+count; pos < end ; pos++)
   {
     enum enum_thr_lock_result result= thr_lock(*pos, owner, (*pos)->type);
     if (result != THR_LOCK_SUCCESS)
     {						/* Aborted */
-      thr_multi_unlock(data,(uint) (pos-data));
+      thr_multi_unlock(data,(uint) (pos-data), 0);
       DBUG_RETURN(result);
     }
 #ifdef MAIN
@@ -1018,63 +1063,103 @@ thr_multi_lock(THR_LOCK_DATA **data, uint count, THR_LOCK_OWNER *owner)
 	   (long) pos[0]->lock, pos[0]->type); fflush(stdout);
 #endif
   }
+
   /*
-    Ensure that all get_locks() have the same status
+    Call start_trans for all locks.
     If we lock the same table multiple times, we must use the same
-    status_param!
+    status_param; We ensure this by calling copy_status() for all
+    copies of the same tables.
   */
-#if !defined(DONT_USE_RW_LOCKS)
-  if (count > 1)
+  if ((*data)->lock->start_trans)
+    ((*data)->lock->start_trans)((*data)->status_param);
+  for (first_lock=data, pos= data+1 ; pos < end ; pos++)
   {
-    THR_LOCK_DATA *last_lock= end[-1];
-    pos=end-1;
-    do
+    /* Get the current status (row count, checksum, trid etc) */
+    if ((*pos)->lock->start_trans)
+      (*(*pos)->lock->start_trans)((*pos)->status_param);
+    /*
+      If same table as previous table use pointer to previous status
+      information to ensure that all read/write tables shares same
+      state.
+    */
+    if (pos[0]->lock == pos[-1]->lock && pos[0]->lock->copy_status)
+      (pos[0]->lock->copy_status)((*pos)->status_param,
+                                  (*first_lock)->status_param);
+    else
     {
-      pos--;
-      if (last_lock->lock == (*pos)->lock &&
-	  last_lock->lock->copy_status)
-      {
-	if (last_lock->type <= TL_READ_NO_INSERT)
-	{
-	  THR_LOCK_DATA **read_lock;
-	  /*
-	    If we are locking the same table with read locks we must ensure
-	    that all tables share the status of the last write lock or
-	    the same read lock.
-	  */
-	  for (;
-	       (*pos)->type <= TL_READ_NO_INSERT &&
-		 pos != data &&
-		 pos[-1]->lock == (*pos)->lock ;
-	       pos--) ;
-
-	  read_lock = pos+1;
-	  do
-	  {
-	    (last_lock->lock->copy_status)((*read_lock)->status_param,
-					   (*pos)->status_param);
-	  } while (*(read_lock++) != last_lock);
-	  last_lock= (*pos);			/* Point at last write lock */
-	}
-	else
-	  (*last_lock->lock->copy_status)((*pos)->status_param,
-					  last_lock->status_param);
-      }
-      else
-	last_lock=(*pos);
-    } while (pos != data);
+      /* Different lock, use this as base for next lock */
+      first_lock= pos;
+    }
   }
-#endif
   DBUG_RETURN(THR_LOCK_SUCCESS);
 }
 
-  /* free all locks */
 
-void thr_multi_unlock(THR_LOCK_DATA **data,uint count)
+/**
+  Merge two sets of locks.
+
+  @param data       All locks. First old locks, then new locks.
+  @param old_count  Original number of locks. These are first in 'data'.
+  @param new_count  How many new locks
+
+  The merge is needed if the new locks contains same tables as the old
+  locks, in which case we have to ensure that same tables shares the
+  same status (as after a thr_multi_lock()).
+*/
+
+void thr_merge_locks(THR_LOCK_DATA **data, uint old_count, uint new_count)
+{
+  THR_LOCK_DATA **pos, **end, **first_lock= 0;
+  DBUG_ENTER("thr_merge_lock");
+
+  /* Remove marks on old locks to make them sort before new ones */
+  for (pos=data, end= pos + old_count; pos < end ; pos++)
+    (*pos)->priority&= ~THR_LOCK_LATE_PRIV;
+
+  /* Mark new locks with LATE_PRIV to make them sort after org ones */
+  for (pos=data + old_count, end= pos + new_count; pos < end ; pos++)
+    (*pos)->priority|= THR_LOCK_LATE_PRIV;
+
+  sort_locks(data, old_count + new_count);
+
+  for (pos=data ; pos < end ; pos++)
+  {
+    /* Check if lock was unlocked before */
+    if (pos[0]->type == TL_UNLOCK || ! pos[0]->lock->fix_status)
+    {
+      DBUG_PRINT("info", ("lock skipped.  unlocked: %d  fix_status: %d",
+                          pos[0]->type == TL_UNLOCK,
+                          pos[0]->lock->fix_status == 0));
+      continue;
+    }
+
+    /*
+      If same table as previous table use pointer to previous status
+      information to ensure that all read/write tables shares same
+      state.
+    */
+    if (first_lock && pos[0]->lock == first_lock[0]->lock)
+      (pos[0]->lock->fix_status)((*first_lock)->status_param,
+                                 (*pos)->status_param);
+    else
+    {
+      /* Different lock, use this as base for next lock */
+      first_lock= pos;
+      (pos[0]->lock->fix_status)((*first_lock)->status_param, 0);
+    }
+  }
+  DBUG_VOID_RETURN;
+}
+
+
+/* Unlock all locks */
+
+void thr_multi_unlock(THR_LOCK_DATA **data,uint count, uint unlock_flags)
 {
   THR_LOCK_DATA **pos,**end;
   DBUG_ENTER("thr_multi_unlock");
-  DBUG_PRINT("lock",("data: 0x%lx  count: %d", (long) data, count));
+  DBUG_PRINT("lock",("data: 0x%lx  count: %d  flags: %u", (long) data, count,
+                     unlock_flags));
 
   for (pos=data,end=data+count; pos < end ; pos++)
   {
@@ -1084,7 +1169,7 @@ void thr_multi_unlock(THR_LOCK_DATA **data,uint count)
     fflush(stdout);
 #endif
     if ((*pos)->type != TL_UNLOCK)
-      thr_unlock(*pos);
+      thr_unlock(*pos, unlock_flags);
     else
     {
       DBUG_PRINT("lock",("Free lock: data: 0x%lx  thread: 0x%lx  lock: 0x%lx",
@@ -1400,6 +1485,7 @@ my_bool thr_upgrade_write_delay_lock(THR_LOCK_DATA *data,
                                      enum thr_lock_type new_lock_type)
 {
   THR_LOCK *lock=data->lock;
+  enum enum_thr_lock_result res;
   DBUG_ENTER("thr_upgrade_write_delay_lock");
 
   pthread_mutex_lock(&lock->mutex);
@@ -1420,6 +1506,8 @@ my_bool thr_upgrade_write_delay_lock(THR_LOCK_DATA *data,
       if (lock->get_status)
 	(*lock->get_status)(data->status_param, 0);
       pthread_mutex_unlock(&lock->mutex);
+      if (lock->start_trans)
+	(*lock->start_trans)(data->status_param);
       DBUG_RETURN(0);
     }
 
@@ -1440,7 +1528,10 @@ my_bool thr_upgrade_write_delay_lock(THR_LOCK_DATA *data,
   {
     check_locks(lock,"waiting for lock",0);
   }
-  DBUG_RETURN(wait_for_lock(&lock->write_wait,data,1));
+  res= wait_for_lock(&lock->write_wait,data,1);
+  if (res == THR_LOCK_SUCCESS && lock->start_trans)
+    DBUG_RETURN((*lock->start_trans)(data->status_param));
+  DBUG_RETURN(0);
 }
 
 
@@ -1684,7 +1775,7 @@ static void *test_thread(void *arg)
       }
     }
     pthread_mutex_unlock(&LOCK_thread_count);
-    thr_multi_unlock(multi_locks,lock_counts[param]);
+    thr_multi_unlock(multi_locks,lock_counts[param], THR_UNLOCK_UPDATE_STATUS);
   }
 
   printf("Thread %s (%d) ended\n",my_thread_name(),param); fflush(stdout);