BUG#29806 - binlog_innodb.test creates a server log

Stopping mysql server could result in an entry in mysql error
file: "InnoDB: Error: MySQL is freeing a thd".

This happened because InnoDB assumes that the server will never
call external_lock(F_UNLCK) in case external_lock(READ/WRITE)
failed.

Prior to this patch we haven't had strict definition whether
external_lock(F_UNLCK) must be called in case external_lock(READ/WRITE)
fails.

This patch states that we never call external_lock(F_UNLCK) in case
external_lock(READ/WRITE) fails.


mysql-test/suite/binlog/t/disabled.def:
  Re-enabled binlog_innodb and binlog_killed tests.
sql/ha_ndbcluster.cc:
  Restore handler state in case external_lock() failed.
sql/ha_partition.cc:
  Do not call external_lock(F_UNLCK) in case external_lock(READ/WRITE) failed.
sql/lock.cc:
  Do not call external_lock(F_UNLCK) in case external_lock(READ/WRITE) failed.
storage/myisammrg/myrg_locking.c:
  Restore handler state in case external_lock() failed.

mysql-test/suite/binlog/t/disabled.def

@@ -9,6 +9,3 @@
 #  Do not use any TAB characters for whitespace.
 #
 ##############################################################################
-
-binlog_innodb        : Bug#29806 2007-07-15 ingo master.err: InnoDB: Error: MySQL is freeing a thd
-binlog_killed        : Bug#29806 2007-07-17 ingo master.err: InnoDB: Error: MySQL is freeing a thd

sql/ha_ndbcluster.cc

@@ -4383,7 +4383,10 @@ int ha_ndbcluster::external_lock(THD *thd, int lock_type)
 
         trans= ndb->startTransaction();
         if (trans == NULL)
+        {
+          thd_ndb->lock_count= 0;
           ERR_RETURN(ndb->getNdbError());
+        }
         thd_ndb->init_open_tables();
         thd_ndb->stmt= trans;
 	thd_ndb->query_state&= NDB_QUERY_NORMAL;
@@ -4409,7 +4412,10 @@ int ha_ndbcluster::external_lock(THD *thd, int lock_type)
           
           trans= ndb->startTransaction();
           if (trans == NULL)
+          {
+            thd_ndb->lock_count= 0;
             ERR_RETURN(ndb->getNdbError());
+          }
           thd_ndb->init_open_tables();
           thd_ndb->all= trans; 
 	  thd_ndb->query_state&= NDB_QUERY_NORMAL;

sql/ha_partition.cc

@@ -1142,6 +1142,12 @@ int ha_partition::prepare_new_partition(TABLE *table,
   create_flag= TRUE;
   if ((error= file->ha_open(table, part_name, m_mode, m_open_test_lock)))
     goto error;
+  /*
+    Note: if you plan to add another call that may return failure,
+    better to do it before external_lock() as cleanup_new_partition()
+    assumes that external_lock() is last call that may fail here.
+    Otherwise see description for cleanup_new_partition().
+  */
   if ((error= file->external_lock(current_thd, m_lock_type)))
     goto error;
 
@@ -1164,6 +1170,14 @@ int ha_partition::prepare_new_partition(TABLE *table,
     NONE
 
   DESCRIPTION
+    This function is called immediately after prepare_new_partition() in
+    case the latter fails.
+
+    In prepare_new_partition() last call that may return failure is
+    external_lock(). That means if prepare_new_partition() fails,
+    partition does not have external lock. Thus no need to call
+    external_lock(F_UNLCK) here.
+
   TODO:
     We must ensure that in the case that we get an error during the process
     that we call external_lock with F_UNLCK, close the table and delete the
@@ -1182,7 +1196,6 @@ void ha_partition::cleanup_new_partition(uint part_count)
     m_file= m_added_file;
     m_added_file= NULL;
 
-    external_lock(current_thd, F_UNLCK);
     /* delete_table also needed, a bit more complex */
     close();
 

sql/lock.cc

@@ -60,6 +60,11 @@
   - When calling UNLOCK TABLES we call mysql_unlock_tables() for all
     tables used in LOCK TABLES
 
+  If table_handler->external_lock(thd, locktype) fails, we call
+  table_handler->external_lock(thd, F_UNLCK) for each table that was locked,
+  excluding one that caused failure. That means handler must cleanup itself
+  in case external_lock() fails.
+
 TODO:
   Change to use my_malloc() ONLY when using LOCK TABLES command or when
   we are forced to use mysql_lock_merge.
@@ -269,8 +274,9 @@ static int lock_external(THD *thd, TABLE **tables, uint count)
     if ((error=(*tables)->file->ha_external_lock(thd,lock_type)))
     {
       print_lock_error(error, (*tables)->file->table_type());
-      for (; i-- ; tables--)
+      while (--i)
       {
+        tables--;
 	(*tables)->file->ha_external_lock(thd, F_UNLCK);
 	(*tables)->current_lock=F_UNLCK;
       }

storage/myisammrg/myrg_locking.c

@@ -37,7 +37,15 @@ int myrg_lock_database(MYRG_INFO *info, int lock_type)
     (file->table)->owned_by_merge = TRUE;
 #endif
     if ((new_error=mi_lock_database(file->table,lock_type)))
+    {
       error=new_error;
+      if (lock_type != F_UNLCK)
+      {
+        while (--file >= info->open_tables)
+          mi_lock_database(file->table, F_UNLCK);
+        break;
+      }
+    }
   }
   return(error);
 }