bzr commit into mysql-6.0-bugfixing branch (jon.hauglid:2840) Bug#46678

[Jon Olav Hauglid]

#At file:///export/home/z/mysql-6.0-codebase-bugfixing-bug46678/ based on revid:li-bing.song@stripped

 2840 Jon Olav Hauglid	2009-09-25
      Bug #46678 Transactional locks do not protect a table from 
                 concurrent DROP or RENAME 
      
      The problem was that transactional LOCK TABLE did not protect 
      the table(s) from DDL operations issued from other connections. 
      Transactional LOCK TABLE is implemented using InnoDB table 
      level locks. But by design, InnoDB ignores these locks for DDL 
      operations.
      
      This patch changes the implementation of transactional LOCK TABLE
      so that affected tables are also kept locked using MDL (MDL_SHARED).
      Before, LOCK TABLE released it's MDL locks at the end of the 
      statement.
      
      Test case added to locktrans_innodb.test.

    modified:
      mysql-test/r/locktrans_innodb.result
      mysql-test/t/locktrans_innodb.test
      sql/lock.cc
=== modified file 'mysql-test/r/locktrans_innodb.result'
--- a/mysql-test/r/locktrans_innodb.result	2009-09-16 13:31:23 +0000
+++ b/mysql-test/r/locktrans_innodb.result	2009-09-25 10:48:44 +0000
@@ -935,3 +935,22 @@ UNLOCK TABLES;
 SET AUTOCOMMIT= 1;
 UNLOCK TABLES;
 DROP TABLE t1, t2;
+#
+# Bug 46678 Transactional locks do not protect a table from 
+#           concurrent DROP or RENAME 
+#
+DROP TABLE IF EXISTS t1;
+CREATE TABLE t1 ( i INT ) engine = innodb;
+# Connection 1
+BEGIN;
+LOCK TABLES t1 IN EXCLUSIVE MODE;
+# Connection 2
+# DROP TABLE should wait
+DROP TABLE t1;
+# Connection 1
+# t1 should still exist
+SELECT * FROM t1;
+i
+COMMIT;
+# Connection 2
+# Connection 1

=== modified file 'mysql-test/t/locktrans_innodb.test'
--- a/mysql-test/t/locktrans_innodb.test	2007-02-13 17:39:23 +0000
+++ b/mysql-test/t/locktrans_innodb.test	2009-09-25 10:48:44 +0000
@@ -18,3 +18,42 @@ let $nowait_support= 1;
 let $other_non_trans_engine_type= MyISAM;
 
 --source include/locktrans.inc
+
+
+--echo #
+--echo # Bug 46678 Transactional locks do not protect a table from 
+--echo #           concurrent DROP or RENAME 
+--echo #
+
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+--enable_warnings
+
+CREATE TABLE t1 ( i INT ) engine = innodb;
+
+--echo # Connection 1
+BEGIN;
+LOCK TABLES t1 IN EXCLUSIVE MODE;
+
+--echo # Connection 2
+connect(con2,localhost,root);
+connection con2;
+--echo # DROP TABLE should wait
+--send DROP TABLE t1
+
+--echo # Connection 1
+connection default;
+let $wait_condition= SELECT COUNT(*)= 1 FROM information_schema.processlist
+                     WHERE state= 'Waiting for table' AND
+                           info= 'DROP TABLE t1';
+--source include/wait_condition.inc
+--echo # t1 should still exist
+SELECT * FROM t1;
+COMMIT;
+
+--echo # Connection 2
+connection con2;
+--reap
+
+--echo # Connection 1
+disconnect con2;

=== modified file 'sql/lock.cc'
--- a/sql/lock.cc	2009-09-16 14:26:50 +0000
+++ b/sql/lock.cc	2009-09-25 10:48:44 +0000
@@ -1511,10 +1511,13 @@ int try_transactional_lock(THD *thd, TAB
 {
   uint          dummy_counter;
   int           error;
-  int           result= 0;
   DBUG_ENTER("try_transactional_lock");
 
-  /* Need to open the tables to be able to access engine methods. */
+  /*
+     Need to open the tables to be able to access engine methods.
+     This will also take MDL_SHARED locks to prevent ddl operations
+     from other connections.
+  */
   if (open_tables(thd, &table_list, &dummy_counter, 0))
   {
     /* purecov: begin tested */
@@ -1526,37 +1529,32 @@ int try_transactional_lock(THD *thd, TAB
   /* Required by InnoDB. */
   thd->in_lock_tables= TRUE;
 
-  if ((error= set_handler_table_locks(thd, table_list, TRUE)))
+  error= set_handler_table_locks(thd, table_list, TRUE);
+  if (error == HA_ERR_WRONG_COMMAND)
   {
     /*
-      Not all transactional locks could be taken. If the error was
-      something else but "unsupported by storage engine", abort the
-      execution of this statement.
-    */
-    if (error != HA_ERR_WRONG_COMMAND)
-    {
-      DBUG_PRINT("lock_info", ("aborting, lock_table failed"));
-      result= -1;
-      goto err;
-    }
-    /*
       Fall back to non-transactional locks because transactional locks
       are unsupported by a storage engine. No need to unlock the
       successfully taken transactional locks. They go away at end of
       transaction anyway.
     */
     DBUG_PRINT("lock_info", ("fall back to non-trans lock: no SE support"));
-    result= 1;
+    /* Tables will be reopened when taking non-transactional locks. */
+    close_tables_for_reopen(thd, &table_list);
+  }
+  else
+  {
+    /* We need to explicitly commit if autocommit mode is active. */
+    trans_commit_stmt(thd);
+    /*
+       Close the tables. The locks (if taken) persist in the storage engines
+       and in the MDL subsystem.
+    */
+    close_thread_tables(thd);
   }
 
- err:
-  /* We need to explicitly commit if autocommit mode is active. */
-  trans_commit_stmt(thd);
-  /* Close the tables. The locks (if taken) persist in the storage engines. */
-  close_tables_for_reopen(thd, &table_list);
   thd->in_lock_tables= FALSE;
-  DBUG_PRINT("lock_info", ("result: %d", result));
-  DBUG_RETURN(result);
+  DBUG_RETURN(error == HA_ERR_WRONG_COMMAND ? 1 : -test(error));
 }
 
 


Attachment: [text/bzr-bundle] bzr/[e-mail]