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.

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;
 }
 

sql/sql_class.h

@@ -2682,6 +2682,12 @@ class THD :public Statement,
   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);

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;
   }

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 @@ bool mysql_ha_read(THD *thd, TABLE_LIST *tables,
     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 @@ bool mysql_ha_read(THD *thd, TABLE_LIST *tables,
   */
   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())
   {

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)