Backport of:

-------------------------------------------------------------------
revno: 2630.6.6
committer: Konstantin Osipov <konstantin@mysql.com>
branch nick: mysql-6.0-3726
timestamp: Tue 2008-05-27 16:15:44 +0400
message:
Implement code review fixes for WL#3726 "DDL locking for all
metadata objects": cleanup the code from share->mutex
acquisitions, which are now obsolete.

sql/ha_partition.cc:
  Rename share->mutex to share->LOCK_ha_data. The mutex never
  protected the entire share. The right way to protect a share
  is to acquire an MDL lock.
sql/ha_partition.h:
  Rename share->mutex to share->LOCK_ha_data.
sql/sql_base.cc:
  Remove LOCK_table_share. Do not acquire share->mutex when
  deleting a table share or incrementing its ref_count.
  All these operations are and must continue to be protected by LOCK_open
  and respective MDL locks.
sql/sql_view.cc:
  Remove acquisition of share->mutex when incrementing share->ref_count.
sql/table.cc:
  Simplify release_table_shar() by removing dead code related
  to share->mutex from it.
sql/table.h:
  Rename TABLE_SHARE::mutex to TABLE_SHARE::LOCK_ha_data
  to better reflect its purpose.
storage/myisam/ha_myisam.cc:
  Rename share->mutex to share->LOCK_ha_data.

sql/ha_partition.cc

@@ -2606,7 +2606,7 @@ int ha_partition::open(const char *name, int mode, uint test_if_locked)
     for the same table.
   */
   if (is_not_tmp_table)
-    pthread_mutex_lock(&table_share->mutex);
+    pthread_mutex_lock(&table_share->LOCK_ha_data);
   if (!table_share->ha_data)
   {
     HA_DATA_PARTITION *ha_data;
@@ -2617,7 +2617,7 @@ int ha_partition::open(const char *name, int mode, uint test_if_locked)
     if (!ha_data)
     {
       if (is_not_tmp_table)
-        pthread_mutex_unlock(&table_share->mutex);
+        pthread_mutex_unlock(&table_share->LOCK_ha_data);
       goto err_handler;
     }
     DBUG_PRINT("info", ("table_share->ha_data 0x%p", ha_data));
@@ -2626,7 +2626,7 @@ int ha_partition::open(const char *name, int mode, uint test_if_locked)
     pthread_mutex_init(&ha_data->mutex, MY_MUTEX_INIT_FAST);
   }
   if (is_not_tmp_table)
-    pthread_mutex_unlock(&table_share->mutex);
+    pthread_mutex_unlock(&table_share->LOCK_ha_data);
   /*
     Some handlers update statistics as part of the open call. This will in
     some cases corrupt the statistics of the partition handler and thus

sql/ha_partition.h

@@ -933,7 +933,7 @@ private:
     if(table_share->tmp_table == NO_TMP_TABLE)
     {
       auto_increment_lock= TRUE;
-      pthread_mutex_lock(&table_share->mutex);
+      pthread_mutex_lock(&table_share->LOCK_ha_data);
     }
   }
   virtual void unlock_auto_increment()
@@ -946,7 +946,7 @@ private:
     */
     if(auto_increment_lock && !auto_increment_safe_stmt_log_lock)
     {
-      pthread_mutex_unlock(&table_share->mutex);
+      pthread_mutex_unlock(&table_share->LOCK_ha_data);
       auto_increment_lock= FALSE;
     }
   }

sql/sql_base.cc

@@ -112,7 +112,6 @@ uint  table_cache_count= 0;
 TABLE *unused_tables;
 HASH table_def_cache;
 static TABLE_SHARE *oldest_unused_share, end_of_unused_share;
-static pthread_mutex_t LOCK_table_share;
 static bool table_def_inited= 0;
 
 static bool check_and_update_table_version(THD *thd, TABLE_LIST *tables,
@@ -253,13 +252,12 @@ extern "C" uchar *table_def_key(const uchar *record, size_t *length,
 static void table_def_free_entry(TABLE_SHARE *share)
 {
   DBUG_ENTER("table_def_free_entry");
+  safe_mutex_assert_owner(&LOCK_open);
   if (share->prev)
   {
     /* remove from old_unused_share list */
-    pthread_mutex_lock(&LOCK_table_share);
     *share->prev= share->next;
     share->next->prev= share->prev;
-    pthread_mutex_unlock(&LOCK_table_share);
   }
   free_table_share(share);
   DBUG_VOID_RETURN;
@@ -269,7 +267,6 @@ static void table_def_free_entry(TABLE_SHARE *share)
 bool table_def_init(void)
 {
   table_def_inited= 1;
-  pthread_mutex_init(&LOCK_table_share, MY_MUTEX_INIT_FAST);
   oldest_unused_share= &end_of_unused_share;
   end_of_unused_share.prev= &oldest_unused_share;
 
@@ -287,7 +284,6 @@ void table_def_free(void)
     /* Free all open TABLEs first. */
     close_cached_tables(NULL, NULL, FALSE, FALSE);
     table_def_inited= 0;
-    pthread_mutex_destroy(&LOCK_table_share);
     /* Free table definitions. */
     my_hash_free(&table_def_cache);
   }
@@ -473,12 +469,6 @@ TABLE_SHARE *get_table_share(THD *thd, TABLE_LIST *table_list, char *key,
     DBUG_RETURN(0);
   }
 
-  /*
-    Lock mutex to be able to read table definition from file without
-    conflicts
-  */
-  (void) pthread_mutex_lock(&share->mutex);
-
   /*
     We assign a new table id under the protection of the LOCK_open and
     the share's own mutex.  We do this insted of creating a new mutex
@@ -508,7 +498,6 @@ TABLE_SHARE *get_table_share(THD *thd, TABLE_LIST *table_list, char *key,
   share->ref_count++;				// Mark in use
   DBUG_PRINT("exit", ("share: 0x%lx  ref_count: %u",
                       (ulong) share, share->ref_count));
-  (void) pthread_mutex_unlock(&share->mutex);
   DBUG_RETURN(share);
 
 found:
@@ -516,20 +505,15 @@ found:
      We found an existing table definition. Return it if we didn't get
      an error when reading the table definition from file.
   */
-
-  /* We must do a lock to ensure that the structure is initialized */
-  (void) pthread_mutex_lock(&share->mutex);
   if (share->error)
   {
     /* Table definition contained an error */
     open_table_error(share, share->error, share->open_errno, share->errarg);
-    (void) pthread_mutex_unlock(&share->mutex);
     DBUG_RETURN(0);
   }
   if (share->is_view && !(db_flags & OPEN_VIEW))
   {
     open_table_error(share, 1, ENOENT, 0);
-    (void) pthread_mutex_unlock(&share->mutex);
     DBUG_RETURN(0);
   }
 
@@ -540,22 +524,16 @@ found:
       Unlink share from this list
     */
     DBUG_PRINT("info", ("Unlinking from not used list"));
-    pthread_mutex_lock(&LOCK_table_share);
     *share->prev= share->next;
     share->next->prev= share->prev;
     share->next= 0;
     share->prev= 0;
-    pthread_mutex_unlock(&LOCK_table_share);
   }
-  (void) pthread_mutex_unlock(&share->mutex);
 
    /* Free cache if too big */
   while (table_def_cache.records > table_def_size &&
          oldest_unused_share->next)
-  {
-    pthread_mutex_lock(&oldest_unused_share->mutex);
     my_hash_delete(&table_def_cache, (uchar*) oldest_unused_share);
-  }
 
   DBUG_PRINT("exit", ("share: 0x%lx  ref_count: %u",
                       (ulong) share, share->ref_count));
@@ -672,7 +650,6 @@ void release_table_share(TABLE_SHARE *share)
 
   safe_mutex_assert_owner(&LOCK_open);
 
-  pthread_mutex_lock(&share->mutex);
   DBUG_ASSERT(share->ref_count);
   if (!--share->ref_count)
   {
@@ -684,12 +661,10 @@ void release_table_share(TABLE_SHARE *share)
       DBUG_PRINT("info",("moving share to unused list"));
 
       DBUG_ASSERT(share->next == 0);
-      pthread_mutex_lock(&LOCK_table_share);
       share->prev= end_of_unused_share.prev;
       *end_of_unused_share.prev= share;
       end_of_unused_share.prev= &share->next;
       share->next= &end_of_unused_share;
-      pthread_mutex_unlock(&LOCK_table_share);
 
       to_be_deleted= (table_def_cache.records > table_def_size);
     }
@@ -699,9 +674,7 @@ void release_table_share(TABLE_SHARE *share)
   {
     DBUG_PRINT("info", ("Deleting share"));
     my_hash_delete(&table_def_cache, (uchar*) share);
-    DBUG_VOID_RETURN;
   }
-  pthread_mutex_unlock(&share->mutex);
   DBUG_VOID_RETURN;
 }
 
@@ -745,9 +718,8 @@ static void reference_table_share(TABLE_SHARE *share)
 {
   DBUG_ENTER("reference_table_share");
   DBUG_ASSERT(share->ref_count);
-  pthread_mutex_lock(&share->mutex);
+  safe_mutex_assert_owner(&LOCK_open);
   share->ref_count++;
-  pthread_mutex_unlock(&share->mutex);
   DBUG_PRINT("exit", ("share: 0x%lx  ref_count: %u",
                      (ulong) share, share->ref_count));
   DBUG_VOID_RETURN;
@@ -1043,10 +1015,7 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables, bool have_lock,
     free_cache_entry(unused_tables);
   /* Free table shares */
   while (oldest_unused_share->next)
-  {
-    pthread_mutex_lock(&oldest_unused_share->mutex);
     (void) my_hash_delete(&table_def_cache, (uchar*) oldest_unused_share);
-  }
 
   if (!wait_for_refresh)
   {
@@ -8581,11 +8550,8 @@ bool remove_table_from_cache(THD *thd, const char *db, const char *table_name,
       DBUG_PRINT("info", ("share version: %lu  ref_count: %u",
                           share->version, share->ref_count));
       if (share->ref_count == 0)
-      {
-        pthread_mutex_lock(&share->mutex);
         my_hash_delete(&table_def_cache, (uchar*) share);
     }
-    }
 
     if (result && (flags & RTFC_WAIT_OTHER_THREAD_FLAG))
     {
@@ -8722,11 +8688,8 @@ void expel_table_from_cache(THD *leave_thd, const char *db, const char *table_na
   {
     DBUG_ASSERT(leave_thd || share->ref_count == 0);
     if (share->ref_count == 0)
-    {
-      pthread_mutex_lock(&share->mutex);
       my_hash_delete(&table_def_cache, (uchar*) share);
   }
-  }
 }
 
 

sql/sql_view.cc

@@ -1643,10 +1643,8 @@ bool mysql_drop_view(THD *thd, TABLE_LIST *views, enum_drop_mode drop_mode)
     if ((share= get_cached_table_share(view->db, view->table_name)))
     {
       DBUG_ASSERT(share->ref_count == 0);
-      pthread_mutex_lock(&share->mutex);
       share->ref_count++;
       share->version= 0;
-      pthread_mutex_unlock(&share->mutex);
       release_table_share(share);
     }
     query_cache_invalidate3(thd, view, 0);

sql/table.cc

@@ -320,7 +320,7 @@ TABLE_SHARE *alloc_table_share(TABLE_LIST *table_list, char *key,
     share->free_tables.empty();
 
     memcpy((char*) &share->mem_root, (char*) &mem_root, sizeof(mem_root));
-    pthread_mutex_init(&share->mutex, MY_MUTEX_INIT_FAST);
+    pthread_mutex_init(&share->LOCK_ha_data, MY_MUTEX_INIT_FAST);
   }
   DBUG_RETURN(share);
 }
@@ -411,16 +411,9 @@ void free_table_share(TABLE_SHARE *share)
   DBUG_PRINT("enter", ("table: %s.%s", share->db.str, share->table_name.str));
   DBUG_ASSERT(share->ref_count == 0);
 
-  /*
-    If someone is waiting for this to be deleted, inform it about this.
-    Don't do a delete until we know that no one is refering to this anymore.
-  */
+  /* The mutex is initialized only for shares that are part of the TDC */
   if (share->tmp_table == NO_TMP_TABLE)
-  {
-    /* No thread refers to this anymore */
-    pthread_mutex_unlock(&share->mutex);
-    pthread_mutex_destroy(&share->mutex);
-  }
+    pthread_mutex_destroy(&share->LOCK_ha_data);
   my_hash_free(&share->name_hash);
 
   plugin_unlock(NULL, share->db_plugin);

sql/table.h

@@ -312,7 +312,7 @@ struct TABLE_SHARE
   TYPELIB keynames;			/* Pointers to keynames */
   TYPELIB fieldnames;			/* Pointer to fieldnames */
   TYPELIB *intervals;			/* pointer to interval info */
-  pthread_mutex_t mutex;                /* For locking the share  */
+  pthread_mutex_t LOCK_ha_data;         /* To protect access to ha_data */
   TABLE_SHARE *next, **prev;            /* Link to unused shares */
 
   /*

storage/myisam/ha_myisam.cc

@@ -1812,7 +1812,7 @@ int ha_myisam::info(uint flag)
 
     /* Update share */
     if (share->tmp_table == NO_TMP_TABLE)
-      pthread_mutex_lock(&share->mutex);
+      pthread_mutex_lock(&share->LOCK_ha_data);
     share->keys_in_use.set_prefix(share->keys);
     share->keys_in_use.intersect_extended(misam_info.key_map);
     share->keys_for_keyread.intersect(share->keys_in_use);
@@ -1822,7 +1822,7 @@ int ha_myisam::info(uint flag)
 	     (char*) misam_info.rec_per_key,
              sizeof(table->key_info[0].rec_per_key[0])*share->key_parts);
     if (share->tmp_table == NO_TMP_TABLE)
-      pthread_mutex_unlock(&share->mutex);
+      pthread_mutex_unlock(&share->LOCK_ha_data);
 
    /*
      Set data_file_name and index_file_name to point at the symlink value