WL#4441 "LOCK_open: Remove requirement of mutex protecting

thd->open_tables"

thd->open_tables list is not normally accessed concurrently
except for one case: when the connection has open SQL
HANDLER tables, and we want to perform a DDL on the table,
we want to abort waits on MyISAM thr_lock of those connections
that prevent the DDL from proceeding, and iterate
over thd->open_tables list to find out the tables on which
the thread is waiting.

In 5.5 we mostly use deadlock detection and soft deadlock 
prevention, as opposed to "hard" deadlock prevention
of 5.1, which would abort any transaction that
may cause a deadlock. The only remaining case when
neither deadlock detection nor deadlock prevention
is implemented in 5.5 is HANDLER SQL, where we use
old good thr_lock_abort() technique form 5.1. 

Thus, replace use of LOCK_open to protect thd->open_tables
with thd->LOCK_ha_data (a lock protecting various session
private data).

This is a port of the work done for 5.5.4 for review
and inclusion into 5.5.5.

5 files changed, 42 insertions, 51 deletions

diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index 657688499db..3b94388d6b8 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -1294,23 +1294,16 @@ static void close_open_tables(THD *thd)
 
   mysql_mutex_assert_not_owner(&LOCK_open);
 
-  mysql_mutex_lock(&LOCK_open);
-
   DBUG_PRINT("info", ("thd->open_tables: 0x%lx", (long) thd->open_tables));
 
   while (thd->open_tables)
     found_old_table|= close_thread_table(thd, &thd->open_tables);
 
-  /* Free tables to hold down open files */
-  while (table_cache_count > table_cache_size && unused_tables)
-    free_cache_entry(unused_tables);
   if (found_old_table)
   {
     /* Tell threads waiting for refresh that something has happened */
     broadcast_refresh();
   }
-
-  mysql_mutex_unlock(&LOCK_open);
 }
 
 
@@ -1342,13 +1335,6 @@ close_all_tables_for_name(THD *thd, TABLE_SHARE *share,
   memcpy(key, share->table_cache_key.str, key_length);
 
   mysql_mutex_assert_not_owner(&LOCK_open);
-  /*
-    We need to hold LOCK_open while changing the open_tables
-    list, since another thread may work on it.
-    @sa mysql_notify_thread_having_shared_lock()
-  */
-  mysql_mutex_lock(&LOCK_open);
-
   for (TABLE **prev= &thd->open_tables; *prev; )
   {
     TABLE *table= *prev;
@@ -1356,10 +1342,9 @@ close_all_tables_for_name(THD *thd, TABLE_SHARE *share,
     if (table->s->table_cache_key.length == key_length &&
         !memcmp(table->s->table_cache_key.str, key, key_length))
     {
-      /* Inform handler that table will be dropped after close */
-      if (table->db_stat)
-        table->file->extra(HA_EXTRA_PREPARE_FOR_DROP);
-
+      thd->locked_tables_list.unlink_from_list(thd,
+                                               table->pos_in_locked_tables,
+                                               remove_from_locked_tables);
       /*
         Does nothing if the table is not locked.
         This allows one to use this function after a table
@@ -1367,12 +1352,11 @@ close_all_tables_for_name(THD *thd, TABLE_SHARE *share,
       */
       mysql_lock_remove(thd, thd->lock, table);
 
-      thd->locked_tables_list.unlink_from_list(thd,
-                                               table->pos_in_locked_tables,
-                                               remove_from_locked_tables);
-
       /* 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);
       close_thread_table(thd, prev);
     }
     else
@@ -1383,7 +1367,6 @@ close_all_tables_for_name(THD *thd, TABLE_SHARE *share,
   }
   /* We have been removing tables from the table cache. */
   broadcast_refresh();
-  mysql_mutex_unlock(&LOCK_open);
 }
 
 
@@ -1582,17 +1565,22 @@ bool close_thread_table(THD *thd, TABLE **table_ptr)
   DBUG_ENTER("close_thread_table");
   DBUG_ASSERT(table->key_read == 0);
   DBUG_ASSERT(!table->file || table->file->inited == handler::NONE);
-  mysql_mutex_assert_owner(&LOCK_open);
+  mysql_mutex_assert_not_owner(&LOCK_open);
+
+  table->mdl_ticket= NULL;
 
+  mysql_mutex_lock(&thd->LOCK_thd_data);
   *table_ptr=table->next;
+  mysql_mutex_unlock(&thd->LOCK_thd_data);
+
+  mysql_mutex_lock(&LOCK_open);
 
-  table->mdl_ticket= NULL;
   if (table->s->needs_reopen() ||
       thd->version != refresh_version || table->needs_reopen() ||
       table_def_shutdown_in_progress)
   {
     free_cache_entry(table);
-    found_old_table=1;
+    found_old_table= 1;
   }
   else
   {
@@ -1602,10 +1590,17 @@ bool close_thread_table(THD *thd, TABLE **table_ptr)
 
     /* Free memory and reset for next loop */
     free_field_buffers_larger_than(table,MAX_TDC_BLOB_SIZE);
-    
+
     table->file->ha_reset();
     table_def_unuse_table(table);
+    /*
+      We free the least used table, not the subject table,
+      to keep the LRU order.
+    */
+    if (table_cache_count > table_cache_size)
+      free_cache_entry(unused_tables);
   }
+  mysql_mutex_unlock(&LOCK_open);
   DBUG_RETURN(found_old_table);
 }
 
@@ -2229,11 +2224,9 @@ void drop_open_table(THD *thd, TABLE *table, const char *db_name,
     /* Ensure the table is removed from the cache. */
     table->s->version= 0;
 
-    mysql_mutex_lock(&LOCK_open);
     table->file->extra(HA_EXTRA_PREPARE_FOR_DROP);
     close_thread_table(thd, &thd->open_tables);
     quick_rm_table(table_type, db_name, table_name, 0);
-    mysql_mutex_unlock(&LOCK_open);
   }
   DBUG_VOID_RETURN;
 }
@@ -3062,8 +3055,8 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
 
   table->mdl_ticket= mdl_ticket;
 
-  table->next=thd->open_tables;		/* Link into simple list */
-  thd->open_tables=table;
+  table->next= thd->open_tables;		/* Link into simple list */
+  thd->set_open_tables(table);
 
   table->reginfo.lock_type=TL_READ;		/* Assume read */
 
@@ -3403,7 +3396,6 @@ unlink_all_closed_tables(THD *thd, MYSQL_LOCK *lock, size_t reopen_count)
   */
   if (reopen_count)
   {
-    mysql_mutex_lock(&LOCK_open);
     while (reopen_count--)
     {
       /*
@@ -3420,7 +3412,6 @@ unlink_all_closed_tables(THD *thd, MYSQL_LOCK *lock, size_t reopen_count)
       close_thread_table(thd, &thd->open_tables);
     }
     broadcast_refresh();
-    mysql_mutex_unlock(&LOCK_open);
   }
   /* Exclude all closed tables from the LOCK TABLES list. */
   for (TABLE_LIST *table_list= m_locked_tables; table_list; table_list=
@@ -8655,10 +8646,10 @@ bool mysql_notify_thread_having_shared_lock(THD *thd, THD *in_use,
     mysql_mutex_unlock(&in_use->mysys_var->mutex);
     signalled= TRUE;
   }
-  mysql_mutex_lock(&LOCK_open);
 
   if (needs_thr_lock_abort)
   {
+    mysql_mutex_lock(&in_use->LOCK_thd_data);
     for (TABLE *thd_table= in_use->open_tables;
          thd_table ;
          thd_table= thd_table->next)
@@ -8669,10 +8660,11 @@ bool mysql_notify_thread_having_shared_lock(THD *thd, THD *in_use,
         and do not remove such instances from the THD::open_tables
         for some time, during which other thread can see those instances
         (e.g. see partitioning code).
-        */
+      */
       if (!thd_table->needs_reopen())
         signalled|= mysql_lock_abort_for_thread(thd, thd_table);
     }
+    mysql_mutex_unlock(&in_use->LOCK_thd_data);
   }
   /*
     Wake up threads waiting in tdc_wait_for_old_versions().
@@ -8685,7 +8677,6 @@ bool mysql_notify_thread_having_shared_lock(THD *thd, THD *in_use,
     a multi-statement transaction.
   */
   broadcast_refresh();
-  mysql_mutex_unlock(&LOCK_open);
   return signalled;
 }
 
diff --git a/sql/sql_class.h b/sql/sql_class.h
index 8a25112b577..8b5fde2ccff 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -2682,6 +2682,12 @@ public:
   void set_query_and_id(char *query_arg, uint32 query_length_arg,
                         query_id_t new_query_id);
   void set_query_id(query_id_t new_query_id);
+  void set_open_tables(TABLE *open_tables_arg)
+  {
+    mysql_mutex_lock(&LOCK_thd_data);
+    open_tables= open_tables_arg;
+    mysql_mutex_unlock(&LOCK_thd_data);
+  }
   void enter_locked_tables_mode(enum_locked_tables_mode mode_arg)
   {
     DBUG_ASSERT(locked_tables_mode == LTM_NONE);
diff --git a/sql/sql_cursor.cc b/sql/sql_cursor.cc
index 59bf0764ada..ca724ec262f 100644
--- a/sql/sql_cursor.cc
+++ b/sql/sql_cursor.cc
@@ -357,7 +357,7 @@ void
 Sensitive_cursor::reset_thd(THD *thd)
 {
   thd->derived_tables= 0;
-  thd->open_tables= 0;
+  thd->set_open_tables(NULL);
   thd->lock= 0;
   thd->free_list= 0;
   thd->change_list.empty();
@@ -436,7 +436,7 @@ Sensitive_cursor::fetch(ulong num_rows)
               thd->lock == 0);
 
   thd->derived_tables= derived_tables;
-  thd->open_tables= open_tables;
+  thd->set_open_tables(open_tables);
   thd->lock= lock;
   thd->set_query_id(query_id);
   change_list.move_elements_to(&thd->change_list);
@@ -519,14 +519,14 @@ Sensitive_cursor::close()
     TABLE *tmp_derived_tables= thd->derived_tables;
     MYSQL_LOCK *tmp_lock= thd->lock;
 
-    thd->open_tables= open_tables;
+    thd->set_open_tables(open_tables);
     thd->derived_tables= derived_tables;
     thd->lock= lock;
 
     /* Is expected to at least close tables and empty thd->change_list */
     stmt_arena->cleanup_stmt();
 
-    thd->open_tables= tmp_derived_tables;
+    thd->set_open_tables(tmp_derived_tables);
     thd->derived_tables= tmp_derived_tables;
     thd->lock= tmp_lock;
   }
diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc
index 23177b8f51a..d07c7eaa277 100644
--- a/sql/sql_handler.cc
+++ b/sql/sql_handler.cc
@@ -131,13 +131,11 @@ static void mysql_ha_close_table(THD *thd, TABLE_LIST *tables)
     /* Non temporary table. */
     tables->table->file->ha_index_or_rnd_end();
     tables->table->open_by_handler= 0;
-    mysql_mutex_lock(&LOCK_open);
     if (close_thread_table(thd, &tables->table))
     {
       /* Tell threads waiting for refresh that something has happened */
       broadcast_refresh();
     }
-    mysql_mutex_unlock(&LOCK_open);
     thd->mdl_context.release_lock(tables->mdl_request.ticket);
   }
   else if (tables->table)
@@ -278,7 +276,7 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen)
     See open_table() back-off comments for more details.
   */
   backup_open_tables= thd->open_tables;
-  thd->open_tables= NULL;
+  thd->set_open_tables(NULL);
   mdl_savepoint= thd->mdl_context.mdl_savepoint();
 
   /*
@@ -312,7 +310,7 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen)
   if (error)
   {
     close_thread_tables(thd);
-    thd->open_tables= backup_open_tables;
+    thd->set_open_tables(backup_open_tables);
     thd->mdl_context.rollback_to_savepoint(mdl_savepoint);
     if (!reopen)
       my_hash_delete(&thd->handler_tables_hash, (uchar*) hash_tables);
@@ -325,7 +323,7 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen)
     DBUG_PRINT("exit",("ERROR"));
     DBUG_RETURN(TRUE);
   }
-  thd->open_tables= backup_open_tables;
+  thd->set_open_tables(backup_open_tables);
   if (hash_tables->mdl_request.ticket)
   {
     thd->mdl_context.
@@ -559,7 +557,7 @@ retry:
     mysql_lock_tables() needs thd->open_tables to be set correctly to
     be able to handle aborts properly.
   */
-  thd->open_tables= hash_tables->table;
+  thd->set_open_tables(hash_tables->table);
 
 
   sql_handler_lock_error.init();
@@ -575,7 +573,7 @@ retry:
   */
   DBUG_ASSERT(hash_tables->table == thd->open_tables);
   /* Restore previous context. */
-  thd->open_tables= backup_open_tables;
+  thd->set_open_tables(backup_open_tables);
 
   if (sql_handler_lock_error.need_reopen())
   {
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index 6fbee4f856e..843fb5b35d2 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -5429,14 +5429,12 @@ bool mysql_create_like_table(THD* thd, TABLE_LIST* table, TABLE_LIST* src_table,
             goto err;
 
           DBUG_ASSERT(thd->open_tables == table->table);
-          mysql_mutex_lock(&LOCK_open);
           /*
             When opening the table, we ignored the locked tables
             (MYSQL_OPEN_GET_NEW_TABLE). Now we can close the table without
             risking to close some locked table.
           */
           close_thread_table(thd, &thd->open_tables);
-          mysql_mutex_unlock(&LOCK_open);
         }
       }
       else                                      // Case 1
@@ -7490,9 +7488,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
                                                create_info);
 
     DBUG_ASSERT(thd->open_tables == t_table);
-    mysql_mutex_lock(&LOCK_open);
     close_thread_table(thd, &thd->open_tables);
-    mysql_mutex_unlock(&LOCK_open);
     table_list->table= 0;
 
     if (error)