Bug#23713 LOCK TABLES + CREATE TRIGGER + FLUSH TABLES WITH READ LOCK = deadlock

This bug is actually two bugs in one, one of which is CREATE TRIGGER under
LOCK TABLES and the other is CREATE TRIGGER under LOCK TABLES simultaneous
to a FLUSH TABLES WITH READ LOCK (global read lock). Both situations could
lead to a server crash or deadlock.

The first problem arises from the fact that when under LOCK TABLES, if the
table is in the set of locked tables, the table is already open and it doesn't
need to be reopened (not a placeholder). Also in this case, if the table is
not write locked, a exclusive lock can't be acquired because of a possible
deadlock with another thread also holding a (read) lock on the table. The
second issue arises from the fact that one should never wait for a global
read lock if it's holding any locked tables, because the global read lock
is waiting for these tables and this leads to a circular wait deadlock.

The solution for the first case is to check if the table is write locked
and upgraded the write lock to a exclusive lock and fail otherwise for non
write locked tables. Grabbin the exclusive lock in this case also means
to ensure that the table is opened only by the calling thread. The second
issue is partly fixed by not waiting for the global read lock if the thread
is holding any locked tables.

The second issue is only partly addressed in this patch because it turned
out to be much wider and also affects other DDL statements. Reported as
Bug#32395


mysql-test/r/trigger.result:
  Add test case result for Bug#23713
mysql-test/r/trigger_notembedded.result:
  Add test case result for Bug#23713
mysql-test/t/trigger.test:
  Add test case for Bug#23713
mysql-test/t/trigger_notembedded.test:
  Add test case for Bug#23713
sql/mysql_priv.h:
  Locally export wait_while_table_is_used and name_lock_locked_table
  and add flag to mysql_ha_rm_tables to signal that LOCK_open is locked.
sql/sql_base.cc:
  Introduce name_lock_locked_table function and match
  close_old_data_files function declaration and definition.
sql/sql_handler.cc:
  Add flag to mysql_ha_rm_tables to signal that LOCK_open is locked.
sql/sql_rename.cc:
  Fix mysql_ha_rm_tables caller.
sql/sql_table.cc:
  Export wait_while_table_is_used and assert that LOCK_open is locked
  and fix mysql_ha_rm_tables caller.
sql/sql_trigger.cc:
  Upgrade write locked tables to a exclusive lock and fail if
  the table is not write locked. Also, don't wait for the global
  read lock if under LOCK TABLES.

1 files changed, 40 insertions, 2 deletions

diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index 3cc8a685aa9..ba8b7fc1330 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -2199,6 +2199,41 @@ void wait_for_condition(THD *thd, pthread_mutex_t *mutex, pthread_cond_t *cond)
 }
 
 
+/**
+  Exclusively name-lock a table that is already write-locked by the
+  current thread.
+
+  @param thd current thread context
+  @param tables able list containing one table to open.
+
+  @return FALSE on success, TRUE otherwise.
+*/
+
+bool name_lock_locked_table(THD *thd, TABLE_LIST *tables)
+{
+  DBUG_ENTER("name_lock_locked_table");
+
+  /* Under LOCK TABLES we must only accept write locked tables. */
+  tables->table= find_locked_table(thd, tables->db, tables->table_name);
+
+  if (!tables->table)
+    my_error(ER_TABLE_NOT_LOCKED, MYF(0), tables->alias);
+  else if (tables->table->reginfo.lock_type < TL_WRITE_LOW_PRIORITY)
+    my_error(ER_TABLE_NOT_LOCKED_FOR_WRITE, MYF(0), tables->alias);
+  else
+  {
+    /*
+      Ensures that table is opened only by this thread and that no
+      other statement will open this table.
+    */
+    wait_while_table_is_used(thd, tables->table, HA_EXTRA_FORCE_REOPEN);
+    DBUG_RETURN(FALSE);
+  }
+
+  DBUG_RETURN(TRUE);
+}
+
+
 /*
   Open table which is already name-locked by this thread.
 
@@ -3118,6 +3153,9 @@ bool reopen_table(TABLE *table)
           then there is only one table open and locked. This means that
           the function probably has to be adjusted before it can be used
           anywhere outside ALTER TABLE.
+
+    @note Must not use TABLE_SHARE::table_name/db of the table being closed,
+          the strings are used in a loop even after the share may be freed.
 */
 
 void close_data_files_and_morph_locks(THD *thd, const char *db,
@@ -3387,8 +3425,8 @@ bool reopen_tables(THD *thd,bool get_locks,bool in_refresh)
     @param send_refresh  Should we awake waiters even if we didn't close any tables?
 */
 
-void close_old_data_files(THD *thd, TABLE *table, bool morph_locks,
-			  bool send_refresh)
+static void close_old_data_files(THD *thd, TABLE *table, bool morph_locks,
+                                 bool send_refresh)
 {
   bool found= send_refresh;
   DBUG_ENTER("close_old_data_files");