Bug #54332 Deadlock with two connections doing LOCK TABLE+INSERT DELAYED

The problem was that deadlocks involving INSERT DELAYED were not detected.

The reason for this is that two threads are involved in INSERT DELAYED:
the connection thread and the handler thread. The connection thread would
wait while the handler thread acquired locks and opened the table.
In essence, this adds an edge to the wait-for-graph between the 
connection thread and the handler thread that the deadlock detector is
unaware of. Therefore many deadlocks involving INSERT DELAYED were not 
detected.

This patch fixes the problem by having the connection thread acquire the
metadata lock the table before starting the handler thread. This allows the
deadlock detector to detect any possible deadlocks resulting from trying to
acquire a metadata lock the table. If a metadata lock is successfully acquired,
the handler thread is started and given a copy of the ticket representing the
metadata lock. When the handler thread then tries to lock and open the table,
it will find that it already has the metadata lock and therefore not acquire
any new metadata locks.

Test cases added to delayed.test.

mysql-test/r/delayed.result

@@ -345,3 +345,78 @@ CREATE TABLE t1 LIKE t2;
 ERROR 42S01: Table 't1' already exists
 DROP TABLE t2;
 DROP TABLE t1;
+#
+# Bug#54332 Deadlock with two connections doing LOCK TABLE+INSERT DELAYED
+#
+DROP TABLE IF EXISTS t1, t2;
+CREATE TABLE t1 (a INT);
+CREATE TABLE t2 (a INT);
+CREATE TABLE t3 (a INT);
+# Test 1: Using LOCK TABLE
+# Connection con1
+LOCK TABLE t1 WRITE;
+# Connection default
+LOCK TABLE t2 WRITE;
+# Sending:
+INSERT DELAYED INTO t1 VALUES (1);
+# Connection con1
+# Wait until INSERT DELAYED is blocked on table 't1'.
+INSERT DELAYED INTO t2 VALUES (1);
+ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
+UNLOCK TABLES;
+# Connection default
+# Reaping: INSERT DELAYED INTO t1 VALUES (1)
+UNLOCK TABLES;
+# Test 2: Using ALTER TABLE
+START TRANSACTION;
+SELECT * FROM t1 WHERE a=0;
+a
+# Connection con1
+# Sending:
+ALTER TABLE t1 COMMENT 'test';
+# Connection default
+# Wait until ALTER TABLE is blocked on table 't1'.
+INSERT DELAYED INTO t1 VALUES (3);
+ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
+COMMIT;
+# Connection con1
+# Reaping: ALTER TABLE t1 COMMENT 'test'
+# Test 3: Using RENAME TABLE
+# Connection default
+START TRANSACTION;
+INSERT INTO t2 VALUES (1);
+# Connection con1
+# Sending:
+RENAME TABLE t1 to t5, t2 to t4;
+# Connection default
+# Wait until RENAME TABLE is blocked on table 't1'.
+INSERT DELAYED INTO t1 VALUES (4);
+ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
+COMMIT;
+# Connection con1
+# Reaping: RENAME TABLE t1 to t5, t2 to t4
+# Connection default
+# Reverting the renames
+RENAME TABLE t5 to t1, t4 to t2;
+# Test 4: Two INSERT DELAYED on the same table
+START TRANSACTION;
+INSERT INTO t2 VALUES (1);
+# Connection con2
+LOCK TABLE t1 WRITE, t2 WRITE;
+# Connection con1
+# Wait until LOCK TABLE is blocked on table 't2'.
+INSERT DELAYED INTO t1 VALUES (5);
+# Connection default
+# Wait until INSERT DELAYED is blocked on table 't1'.
+INSERT DELAYED INTO t1 VALUES (6);
+ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
+COMMIT;
+# Connection con2
+# Reaping: LOCK TABLE t1 WRITE, t2 WRITE
+UNLOCK TABLES;
+# Connection con1
+# Reaping: INSERT DELAYED INTO t1 VALUES (5)
+# Connection con2
+# Connection con1
+# Connection default
+DROP TABLE t1, t2, t3;

mysql-test/t/delayed.test

@@ -388,3 +388,162 @@ CREATE TABLE t1 LIKE t2;
 
 DROP TABLE t2;
 DROP TABLE t1;
+
+
+--echo #
+--echo # Bug#54332 Deadlock with two connections doing LOCK TABLE+INSERT DELAYED
+--echo #
+
+--disable_warnings
+DROP TABLE IF EXISTS t1, t2;
+--enable_warnings
+
+CREATE TABLE t1 (a INT);
+CREATE TABLE t2 (a INT);
+CREATE TABLE t3 (a INT);
+
+--echo # Test 1: Using LOCK TABLE
+
+--echo # Connection con1
+connect (con1, localhost, root);
+LOCK TABLE t1 WRITE;
+
+--echo # Connection default
+connection default;
+LOCK TABLE t2 WRITE;
+--echo # Sending:
+--send INSERT DELAYED INTO t1 VALUES (1)
+
+--echo # Connection con1
+connection con1;
+--echo # Wait until INSERT DELAYED is blocked on table 't1'.
+let $wait_condition=
+  SELECT COUNT(*) = 1 FROM information_schema.processlist
+  WHERE state = "Waiting for table metadata lock"
+  AND info = "INSERT DELAYED INTO t1 VALUES (1)";
+--source include/wait_condition.inc
+--error ER_LOCK_DEADLOCK
+INSERT DELAYED INTO t2 VALUES (1);
+UNLOCK TABLES;
+
+--echo # Connection default
+connection default;
+--echo # Reaping: INSERT DELAYED INTO t1 VALUES (1)
+--reap
+UNLOCK TABLES;
+
+--echo # Test 2: Using ALTER TABLE
+
+START TRANSACTION;
+SELECT * FROM t1 WHERE a=0;
+
+--echo # Connection con1
+connection con1;
+--echo # Sending:
+--send ALTER TABLE t1 COMMENT 'test'
+
+--echo # Connection default
+connection default;
+--echo # Wait until ALTER TABLE is blocked on table 't1'.
+let $wait_condition=
+  SELECT COUNT(*) = 1 FROM information_schema.processlist
+  WHERE state = "Waiting for table metadata lock"
+  AND info = "ALTER TABLE t1 COMMENT 'test'";
+--source include/wait_condition.inc
+--error ER_LOCK_DEADLOCK
+INSERT DELAYED INTO t1 VALUES (3);
+COMMIT;
+
+--echo # Connection con1
+connection con1;
+--echo # Reaping: ALTER TABLE t1 COMMENT 'test'
+--reap
+
+--echo # Test 3: Using RENAME TABLE
+
+--echo # Connection default
+connection default;
+START TRANSACTION;
+INSERT INTO t2 VALUES (1);
+
+--echo # Connection con1
+connection con1;
+--echo # Sending:
+--send RENAME TABLE t1 to t5, t2 to t4
+
+--echo # Connection default
+connection default;
+--echo # Wait until RENAME TABLE is blocked on table 't1'.
+let $wait_condition=
+  SELECT COUNT(*) = 1 FROM information_schema.processlist
+  WHERE state = "Waiting for table metadata lock"
+  AND info = "RENAME TABLE t1 to t5, t2 to t4";
+--source include/wait_condition.inc
+--error ER_LOCK_DEADLOCK
+INSERT DELAYED INTO t1 VALUES (4);
+COMMIT;
+
+--echo # Connection con1
+connection con1;
+--echo # Reaping: RENAME TABLE t1 to t5, t2 to t4
+--reap
+
+--echo # Connection default
+connection default;
+--echo # Reverting the renames
+RENAME TABLE t5 to t1, t4 to t2;
+
+--echo # Test 4: Two INSERT DELAYED on the same table
+
+START TRANSACTION;
+INSERT INTO t2 VALUES (1);
+
+--echo # Connection con2
+connect (con2, localhost, root);
+--send LOCK TABLE t1 WRITE, t2 WRITE
+
+--echo # Connection con1
+connection con1;
+--echo # Wait until LOCK TABLE is blocked on table 't2'.
+let $wait_condition=
+  SELECT COUNT(*) = 1 FROM information_schema.processlist
+  WHERE state = "Waiting for table metadata lock"
+  AND info = "LOCK TABLE t1 WRITE, t2 WRITE";
+--source include/wait_condition.inc
+--send INSERT DELAYED INTO t1 VALUES (5)
+
+--echo # Connection default
+connection default;
+--echo # Wait until INSERT DELAYED is blocked on table 't1'.
+let $wait_condition=
+  SELECT COUNT(*) = 1 FROM information_schema.processlist
+  WHERE state = "Waiting for table metadata lock"
+  AND info = "INSERT DELAYED INTO t1 VALUES (5)";
+--source include/wait_condition.inc
+--error ER_LOCK_DEADLOCK
+INSERT DELAYED INTO t1 VALUES (6);
+COMMIT;
+
+--echo # Connection con2
+connection con2;
+--echo # Reaping: LOCK TABLE t1 WRITE, t2 WRITE
+--reap
+UNLOCK TABLES;
+
+--echo # Connection con1
+connection con1;
+--echo # Reaping: INSERT DELAYED INTO t1 VALUES (5)
+--reap
+
+--echo # Connection con2
+connection con2;
+disconnect con2;
+--source include/wait_until_disconnected.inc
+--echo # Connection con1
+connection con1;
+disconnect con1;
+--source include/wait_until_disconnected.inc
+
+--echo # Connection default
+connection default;
+DROP TABLE t1, t2, t3;

sql/sql_insert.cc

@@ -548,10 +548,34 @@ bool open_and_lock_for_insert_delayed(THD *thd, TABLE_LIST *table_list)
     DBUG_RETURN(TRUE);
   }
 
-  if (delayed_get_table(thd, table_list))
+  /*
+    In order for the deadlock detector to be able to find any deadlocks
+    caused by the handler thread locking this table, we take the metadata
+    lock inside the connection thread. If this goes ok, the ticket is cloned
+    and added to the list of granted locks held by the handler thread.
+  */
+  MDL_ticket *mdl_savepoint= thd->mdl_context.mdl_savepoint();
+  if (thd->mdl_context.acquire_lock(&table_list->mdl_request,
+                                    thd->variables.lock_wait_timeout))
+    /*
+      If a lock can't be acquired, it makes no sense to try normal insert.
+      Therefore we just abort the statement.
+    */
     DBUG_RETURN(TRUE);
 
-  if (table_list->table)
+  /*
+    If a lock was acquired above, we should release it after delayed_get_table()
+    has cloned the ticket for the handler thread. Note that acquire_lock() can
+    succeed because of a lock already held by the connection. In this case we
+    should not release it here.
+  */
+  MDL_ticket *table_ticket = mdl_savepoint == thd->mdl_context.mdl_savepoint() ?
+    NULL: thd->mdl_context.mdl_savepoint();
+
+  bool error= FALSE;
+  if (delayed_get_table(thd, table_list))
+    error= TRUE;
+  else if (table_list->table)
   {
     /*
       Open tables used for sub-selects or in stored functions, will also
@@ -560,16 +584,30 @@ bool open_and_lock_for_insert_delayed(THD *thd, TABLE_LIST *table_list)
     if (open_and_lock_tables(thd, table_list->next_global, TRUE, 0))
     {
       end_delayed_insert(thd);
-      DBUG_RETURN(TRUE);
+      error= TRUE;
+    }
+    else
+    {
+      /*
+        First table was not processed by open_and_lock_tables(),
+        we need to set updatability flag "by hand".
+      */
+      if (!table_list->derived && !table_list->view)
+        table_list->updatable= 1;  // usual table
     }
-    /*
-      First table was not processed by open_and_lock_tables(),
-      we need to set updatability flag "by hand".
-    */
-    if (!table_list->derived && !table_list->view)
-      table_list->updatable= 1;  // usual table
-    DBUG_RETURN(FALSE);
   }
+
+  if (table_ticket)
+    thd->mdl_context.release_lock(table_ticket);
+  /*
+    Clone_ticket() in delayed_get_table() causes TABLE_LIST::MDL_REQUEST::ticket
+    to be overwritten with the cloned ticket. Reset the ticket here in case
+    we end up having to use normal insert.
+  */
+  table_list->mdl_request.ticket= NULL;
+
+  if (error || table_list->table)
+    DBUG_RETURN(error);
 #endif
   /*
     * This is embedded library and we don't have auxiliary
@@ -2025,6 +2063,20 @@ bool delayed_get_table(THD *thd, TABLE_LIST *table_list)
       /* Replace volatile strings with local copies */
       di->table_list.alias= di->table_list.table_name= di->thd.query();
       di->table_list.db= di->thd.db;
+
+      /*
+        Clone the ticket representing the lock on the target table for
+        the insert and add it to the list of granted metadata locks held by
+        the handler thread. This is safe since the handler thread is
+        not holding nor waiting on any metadata locks.
+      */
+      if (di->thd.mdl_context.clone_ticket(&table_list->mdl_request))
+      {
+        delete di;
+        my_error(ER_OUT_OF_RESOURCES, MYF(ME_FATALERROR));
+        goto end_create;
+      }
+
       di->lock();
       mysql_mutex_lock(&di->mutex);
       if ((error= mysql_thread_create(key_thread_delayed_insert,
@@ -2036,6 +2088,7 @@ bool delayed_get_table(THD *thd, TABLE_LIST *table_list)
 		    error));
         mysql_mutex_unlock(&di->mutex);
 	di->unlock();
+        di->thd.mdl_context.release_lock(table_list->mdl_request.ticket);
 	delete di;
 	my_error(ER_CANT_CREATE_THREAD, MYF(ME_FATALERROR), error);
         goto end_create;