Bug #29154: LOCK TABLES is not atomic when >1 InnoDB tables are locked

  LOCK TABLES takes a list of tables to lock. It may lock several 
  tables successfully and then encounter a tables that it can't lock, 
  e.g. because it's locked. In such case it needs to undo the locks on
  the already locked tables. And it does that. But it has also notified
  the relevant table storage engine handlers that they should lock.
  The only reliable way to ensure that the table handlers will give up
  their locks is to end the transaction. This is what UNLOCK TABLE 
  does : it ends the transaction if there were locked tables by LOCK 
  tables.
  It is possible to end the transaction when the lock fails in 
  LOCK TABLES because LOCK TABLES ends the transaction at its start 
  already. 
  Fixed by ending (again) the transaction when LOCK TABLES fails to
  lock a table.

mysql-test/r/innodb_mysql.result

@@ -661,4 +661,19 @@ UPDATE t3 SET a = 'us' WHERE a = 'uk';
 SELECT * FROM t3 WHERE a = 'uk';
 a
 DROP TABLE t1,t2,t3;
+CREATE TABLE t1 (a INT) ENGINE=InnoDB;
+CREATE TABLE t2 (a INT) ENGINE=InnoDB;
+switch to connection c1
+SET AUTOCOMMIT=0;
+INSERT INTO t2 VALUES (1);
+switch to connection c2
+SET AUTOCOMMIT=0;
+LOCK TABLES t1 READ, t2 READ;
+ERROR HY000: Lock wait timeout exceeded; try restarting transaction
+switch to connection c1
+COMMIT;
+INSERT INTO t1 VALUES (1);
+switch to connection default
+SET AUTOCOMMIT=default;
+DROP TABLE t1,t2;
 End of 5.0 tests

mysql-test/t/innodb_mysql.test

@@ -636,4 +636,39 @@ SELECT * FROM t3 WHERE a = 'uk';
 
 DROP TABLE t1,t2,t3;
 
+
+#
+# Bug #29154: LOCK TABLES is not atomic when >1 InnoDB tables are locked
+#
+
+CREATE TABLE t1 (a INT) ENGINE=InnoDB; 
+CREATE TABLE t2 (a INT) ENGINE=InnoDB; 
+
+CONNECT (c1,localhost,root,,);
+CONNECT (c2,localhost,root,,);
+
+--echo switch to connection c1
+CONNECTION c1;
+SET AUTOCOMMIT=0;
+INSERT INTO t2 VALUES (1);
+
+--echo switch to connection c2
+CONNECTION c2;
+SET AUTOCOMMIT=0;
+--error ER_LOCK_WAIT_TIMEOUT
+LOCK TABLES t1 READ, t2 READ;
+
+--echo switch to connection c1
+CONNECTION c1;
+COMMIT;
+INSERT INTO t1 VALUES (1);
+
+--echo switch to connection default
+CONNECTION default;
+SET AUTOCOMMIT=default;
+DISCONNECT c1;
+DISCONNECT c2;
+DROP TABLE t1,t2;
+
+
 --echo End of 5.0 tests

sql/sql_parse.cc

@@ -3837,7 +3837,10 @@ end_with_restore_list:
     break;
   case SQLCOM_LOCK_TABLES:
     unlock_locked_tables(thd);
-    if (check_db_used(thd, all_tables) || end_active_trans(thd))
+    /* we must end the trasaction first, regardless of anything */
+    if (end_active_trans(thd))
+      goto error;
+    if (check_db_used(thd, all_tables))
       goto error;
     if (check_table_access(thd, LOCK_TABLES_ACL | SELECT_ACL, all_tables, 0))
       goto error;
@@ -3855,7 +3858,15 @@ end_with_restore_list:
       send_ok(thd);
     }
     else
+    {
+      /* 
+        Need to end the current transaction, so the storage engine (InnoDB)
+        can free its locks if LOCK TABLES locked some tables before finding
+        that it can't lock a table in its list
+      */
+      end_active_trans(thd);
       thd->options&= ~(ulong) (OPTION_TABLE_LOCK);
+    }
     thd->in_lock_tables=0;
     break;
   case SQLCOM_CREATE_DB: