bk commit into 5.0 tree (davi:1.2510) BUG#28870

[Davi Arnaut]

Below is the list of changes that have just been committed into a local
5.0 repository of davi. When davi does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-09-10 10:28:32-03:00, davi@stripped +1 -0
  Bug#28870 check that table locks are released/reset
  
  The problem is that some mysql_lock_tables error paths are not resetting the
  tables lock type back to TL_UNLOCK. If the lock types are not reset properly,
  a table might be returned to the table cache with wrong lock_type.
  
  The proposed fix is to ensure that the tables lock type is always properly
  reset when mysql_lock_tables fails.

  sql/lock.cc@stripped, 2007-09-10 10:28:28-03:00, davi@stripped +59 -71
    Merge mysql_lock_tables cleanup sequence and the reset_lock_data
    function into a single function and take steps to ensure it is
    always called for each error exit path.

diff -Nrup a/sql/lock.cc b/sql/lock.cc
--- a/sql/lock.cc	2007-08-27 10:13:52 -03:00
+++ b/sql/lock.cc	2007-09-10 10:28:28 -03:00
@@ -77,11 +77,48 @@ extern HASH open_cache;
 
 static MYSQL_LOCK *get_lock_data(THD *thd, TABLE **table,uint count,
 				 uint flags, TABLE **write_locked);
-static void reset_lock_data(MYSQL_LOCK *sql_lock);
 static int lock_external(THD *thd, TABLE **table,uint count);
 static int unlock_external(THD *thd, TABLE **table,uint count);
 static void print_lock_error(int error, const char *);
 
+/**
+  Reset lock type in lock data and free.
+
+  @param mysql_lock Lock structures to reset.
+
+  @note After a locking error we want to quit the locking of the table(s).
+        The test case in the bug report for Bug #18544 has the following
+        cases: 1. Locking error in lock_external() due to InnoDB timeout.
+        2. Locking error in get_lock_data() due to missing write permission.
+        3. Locking error in wait_if_global_read_lock() due to lock conflict.
+
+  @note In all these cases we have already set the lock type into the lock
+        data of the open table(s). If the table(s) are in the open table
+        cache, they could be reused with the non-zero lock type set. This
+        could lead to ignoring a different lock type with the next lock.
+
+  @note Clear the lock type of all lock data. This ensures that the next
+        lock request will set its lock type properly.
+*/
+
+static void reset_lock_data_and_free(MYSQL_LOCK **mysql_lock)
+{
+  MYSQL_LOCK *sql_lock= *mysql_lock;
+  THR_LOCK_DATA **ldata, **ldata_end;
+
+  /* Clear the lock type of all lock data to avoid reusage. */
+  for (ldata= sql_lock->locks, ldata_end= ldata + sql_lock->lock_count;
+       ldata < ldata_end;
+       ldata++)
+  {
+    /* Reset lock type. */
+    (*ldata)->type= TL_UNLOCK;
+  }
+  my_free((gptr) sql_lock, MYF(0));
+  *mysql_lock= 0;
+}
+
+
 /*
   Lock tables.
 
@@ -135,17 +172,12 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd, 
       */
       if (wait_if_global_read_lock(thd, 1, 1))
       {
-        /* Clear the lock type of all lock data to avoid reusage. */
-        reset_lock_data(sql_lock);
-	my_free((gptr) sql_lock,MYF(0));
-	sql_lock=0;
+        reset_lock_data_and_free(&sql_lock);
 	break;
       }
       if (thd->version != refresh_version)
       {
-        /* Clear the lock type of all lock data to avoid reusage. */
-        reset_lock_data(sql_lock);
-	my_free((gptr) sql_lock,MYF(0));
+        reset_lock_data_and_free(&sql_lock);
 	goto retry;
       }
     }
@@ -154,10 +186,7 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd, 
     if (sql_lock->table_count && lock_external(thd, sql_lock->table,
                                                sql_lock->table_count))
     {
-      /* Clear the lock type of all lock data to avoid reusage. */
-      reset_lock_data(sql_lock);
-      my_free((gptr) sql_lock,MYF(0));
-      sql_lock=0;
+      reset_lock_data_and_free(&sql_lock);
       break;
     }
     thd->proc_info="Table lock";
@@ -174,42 +203,43 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd, 
     {
       if (sql_lock->table_count)
         VOID(unlock_external(thd, sql_lock->table, sql_lock->table_count));
+      reset_lock_data_and_free(&sql_lock);
       my_error(rc, MYF(0));
-      my_free((gptr) sql_lock,MYF(0));
-      sql_lock= 0;
       break;
     }
     else if (rc == 1)                           /* aborted */
     {
-      /*
-        reset_lock_data is required here. If thr_multi_lock fails it
-        resets lock type for tables, which were locked before (and
-        including) one that caused error. Lock type for other tables
-        preserved.
-      */
-      reset_lock_data(sql_lock);
       thd->some_tables_deleted=1;		// Try again
       sql_lock->lock_count= 0;                  // Locks are already freed
+      // Fall through: unlock, reset lock data, free and retry
     }
     else if (!thd->some_tables_deleted || (flags & MYSQL_LOCK_IGNORE_FLUSH))
-    {
-      thd->locked=0;
       break;
-    }
     else if (!thd->open_tables)
     {
       // Only using temporary tables, no need to unlock
       thd->some_tables_deleted=0;
-      thd->locked=0;
       break;
     }
     thd->proc_info=0;
 
+    /* going to retry, unlock all tables */
+    if (sql_lock->lock_count)
+        thr_multi_unlock(sql_lock->locks, sql_lock->lock_count);
+
+    if (sql_lock->table_count)
+      VOID(unlock_external(thd, sql_lock->table, sql_lock->table_count));
+
+    /*
+      If thr_multi_lock fails it resets lock type for tables, which
+      were locked before (and including) one that caused error. Lock
+      type for other tables preserved.
+    */
+    reset_lock_data_and_free(&sql_lock);
+
     /* some table was altered or deleted. reopen tables marked deleted */
-    mysql_unlock_tables(thd,sql_lock);
     thd->locked=0;
 retry:
-    sql_lock=0;
     if (flags & MYSQL_LOCK_NOTIFY_IF_NEED_REOPEN)
     {
       *need_reopen= TRUE;
@@ -219,6 +249,7 @@ retry:
       break;					// Couldn't open tables
   }
   thd->proc_info=0;
+  thd->locked= 0;
   if (thd->killed)
   {
     thd->send_kill_message();
@@ -741,8 +772,7 @@ static MYSQL_LOCK *get_lock_data(THD *th
 	my_error(ER_OPEN_AS_READONLY,MYF(0),table->alias);
         /* Clear the lock type of the lock data that are stored already. */
         sql_lock->lock_count= locks - sql_lock->locks;
-        reset_lock_data(sql_lock);
-	my_free((gptr) sql_lock,MYF(0));
+        reset_lock_data_and_free(&sql_lock);
 	DBUG_RETURN(0);
       }
     }
@@ -763,48 +793,6 @@ static MYSQL_LOCK *get_lock_data(THD *th
 	(*org_locks)->debug_print_param= (void *) table;
   }
   DBUG_RETURN(sql_lock);
-}
-
-
-/*
-  Reset lock type in lock data.
-
-  SYNOPSIS
-    reset_lock_data()
-      sql_lock                  The MySQL lock.
-
-  DESCRIPTION
-
-    After a locking error we want to quit the locking of the table(s).
-    The test case in the bug report for Bug #18544 has the following
-    cases: 1. Locking error in lock_external() due to InnoDB timeout.
-    2. Locking error in get_lock_data() due to missing write permission.
-    3. Locking error in wait_if_global_read_lock() due to lock conflict.
-
-    In all these cases we have already set the lock type into the lock
-    data of the open table(s). If the table(s) are in the open table
-    cache, they could be reused with the non-zero lock type set. This
-    could lead to ignoring a different lock type with the next lock.
-
-    Clear the lock type of all lock data. This ensures that the next
-    lock request will set its lock type properly.
-
-  RETURN
-    void
-*/
-
-static void reset_lock_data(MYSQL_LOCK *sql_lock)
-{
-  THR_LOCK_DATA **ldata;
-  THR_LOCK_DATA **ldata_end;
-
-  for (ldata= sql_lock->locks, ldata_end= ldata + sql_lock->lock_count;
-       ldata < ldata_end;
-       ldata++)
-  {
-    /* Reset lock type. */
-    (*ldata)->type= TL_UNLOCK;
-  }
 }