A review comment for WL#4441 " LOCK_open: Remove requirement of

mutex protecting thd->open_tables".

We should not manipulate with table->s->version outside the 
table definition cache code, but use the TDC API
to achieve the desired result.

Fix one violation: close_all_tables_for_name().

sql/sql_base.cc:
  Use tdc_remove_table(TDC_RT_REMOVE_ALL) to expel the
  table share in close_all_tables_for_name, rather
  than manipulate with the table cache explicitly.
  
  Make sure that close_cached_tables() calls 
  close_all_tables_for_name() after closing all the involved
  handlers. The rest of the code was inspected to
  make sure that mysql_ha_rm_tables() is called.
sql/sql_handler.cc:
  Add a method to close all HANDLER cursors for a list
  of tables.
sql/sql_handler.h:
  Add declaration for mysql_ha_flush_tables().

3 files changed, 48 insertions, 6 deletions

diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index 3b94388d6b8..39e6f976f3b 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -1022,6 +1022,9 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables, bool have_lock,
     TABLE_LIST *tables_to_reopen= (tables ? tables :
                                   thd->locked_tables_list.locked_tables());
 
+    /* Close open HANLER instances to avoid self-deadlock. */
+    mysql_ha_flush_tables(thd, tables_to_reopen);
+
     for (TABLE_LIST *table_list= tables_to_reopen; table_list;
          table_list= table_list->next_global)
     {
@@ -1049,7 +1052,8 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables, bool have_lock,
   {
     found= FALSE;
     /*
-      To avoid self and other kinds of deadlock we have to flush open HANDLERs.
+      To a self-deadlock or deadlocks with other FLUSH threads
+      waiting on our open HANDLERs, we have to flush them.
     */
     mysql_ha_flush(thd);
     DEBUG_SYNC(thd, "after_flush_unlock");
@@ -1308,8 +1312,7 @@ static void close_open_tables(THD *thd)
 
 
 /**
-  Close all open instances of the table but keep the MDL lock,
-  if any.
+  Close all open instances of the table but keep the MDL lock.
 
   Works both under LOCK TABLES and in the normal mode.
   Removes all closed instances of the table from the table cache.
@@ -1323,6 +1326,8 @@ static void close_open_tables(THD *thd)
                      In that case the documented behaviour is to
                      implicitly remove the table from LOCK TABLES
                      list.
+
+  @pre Must be called with an X MDL lock on the table.
 */
 
 void
@@ -1331,6 +1336,8 @@ close_all_tables_for_name(THD *thd, TABLE_SHARE *share,
 {
   char key[MAX_DBKEY_LENGTH];
   uint key_length= share->table_cache_key.length;
+  const char *db= key;
+  const char *table_name= db + share->db.length + 1;
 
   memcpy(key, share->table_cache_key.str, key_length);
 
@@ -1352,8 +1359,6 @@ close_all_tables_for_name(THD *thd, TABLE_SHARE *share,
       */
       mysql_lock_remove(thd, thd->lock, table);
 
-      /* Make sure the table is removed from the cache */
-      table->s->version= 0;
       /* Inform handler that table will be dropped after close */
       if (table->db_stat) /* Not true for partitioned tables. */
         table->file->extra(HA_EXTRA_PREPARE_FOR_DROP);
@@ -1365,7 +1370,14 @@ close_all_tables_for_name(THD *thd, TABLE_SHARE *share,
       prev= &table->next;
     }
   }
-  /* We have been removing tables from the table cache. */
+  /* Remove the table share from the cache. */
+  mysql_mutex_lock(&LOCK_open);
+  tdc_remove_table(thd, TDC_RT_REMOVE_ALL, db, table_name);
+  mysql_mutex_unlock(&LOCK_open);
+  /*
+    There could be a FLUSH thread waiting
+    on the table to go away. Wake it up.
+  */
   broadcast_refresh();
 }
 
diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc
index d07c7eaa277..b2e793b5938 100644
--- a/sql/sql_handler.cc
+++ b/sql/sql_handler.cc
@@ -834,6 +834,35 @@ void mysql_ha_rm_tables(THD *thd, TABLE_LIST *tables)
 
 
 /**
+  Close cursors of matching tables from the HANDLER's hash table.
+
+  @param thd Thread identifier.
+  @param tables The list of tables to flush.
+*/
+
+void mysql_ha_flush_tables(THD *thd, TABLE_LIST *all_tables)
+{
+  DBUG_ENTER("mysql_ha_flush_tables");
+
+  for (TABLE_LIST *table_list= all_tables; table_list;
+       table_list= table_list->next_global)
+  {
+    TABLE_LIST *hash_tables= mysql_ha_find(thd, table_list);
+    /* Close all aliases of the same table. */
+    while (hash_tables)
+    {
+      TABLE_LIST *next_local= hash_tables->next_local;
+      if (hash_tables->table)
+        mysql_ha_close_table(thd, hash_tables);
+      hash_tables= next_local;
+    }
+  }
+
+  DBUG_VOID_RETURN;
+}
+
+
+/**
   Flush (close and mark for re-open) all tables that should be should
   be reopen.
 
diff --git a/sql/sql_handler.h b/sql/sql_handler.h
index 8666d5a8d7b..c5da3c4d468 100644
--- a/sql/sql_handler.h
+++ b/sql/sql_handler.h
@@ -28,6 +28,7 @@ bool mysql_ha_close(THD *thd, TABLE_LIST *tables);
 bool mysql_ha_read(THD *, TABLE_LIST *,enum enum_ha_read_modes,char *,
                    List<Item> *,enum ha_rkey_function,Item *,ha_rows,ha_rows);
 void mysql_ha_flush(THD *thd);
+void mysql_ha_flush_tables(THD *thd, TABLE_LIST *all_tables);
 void mysql_ha_rm_tables(THD *thd, TABLE_LIST *tables);
 void mysql_ha_cleanup(THD *thd);
 void mysql_ha_move_tickets_after_trans_sentinel(THD *thd);