bk commit into 4.1 tree (dlenev:1.2388) BUG#12423

[dlenev]

Below is the list of changes that have just been committed into a local
4.1 repository of dlenev. When dlenev does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet
  1.2388 05/09/01 16:52:59 dlenev@stripped +6 -0
  Fix for bug #12423 "Deadlock when doing FLUSH PRIVILEGES and GRANT in 
  multi-threaded environment".
  
  To avoid deadlocks between several simultaneously run account management 
  commands (particularly between FLUSH PRIVILEGES/SET PASSWORD and GRANT
  commands) we should always take table and internal locks during their
  execution in the same order. In other words we should first open and lock
  privilege tables and only then obtain acl_cache::lock/LOCK_grant locks.

  sql/sql_parse.cc
    1.461 05/09/01 16:52:55 dlenev@stripped +21 -4
    If reload_acl_and_cache() is called from SIGHUP handler we have to
    allocate temporary THD for execution of acl_reload()/grant_reload().

  sql/sql_acl.h
    1.33 05/09/01 16:52:55 dlenev@stripped +4 -4
    acl_init/grant_init() are now used only at server start up so they always
    allocate temporary THD object and don't need argument for passing pointer
    to it. acl_reload()/grant_reload() now are able to report about their
    success or failure through return value.

  sql/sql_acl.cc
    1.167 05/09/01 16:52:55 dlenev@stripped +241 -161
    To avoid deadlocks between several simultaneously run account management 
    commands (particularly between FLUSH PRIVILEGES/SET PASSWORD and GRANT
    commands) we should always take table and internal locks during their
    execution in the same order. In other words we should first open and lock
    privilege tables and only then obtain acl_cache::lock/LOCK_grant locks.
    
    Changed acl_reload()/grant_reload() and change_password()/update_user_table()
    in such way that they obey this principle. Now in acl_reload()/grant_reload()/
    change_password() we open and lock privilege tables, then obtain internal
    locks and then call acl_load()/grant_load()/update_user_table() functions to
    do actual loading or updating.

  sql/mysqld.cc
    1.589 05/09/01 16:52:55 dlenev@stripped +2 -2
    acl_init/grant_init() are now used only at server start up so they always
    allocate temporary THD object and don't need argument for passing pointer
    to it.

  mysql-test/t/grant2.test
    1.13 05/09/01 16:52:55 dlenev@stripped +43 -0
    Added test for bug #12423 "Deadlock when doing FLUSH PRIVILEGES and GRANT in 
    multi-threaded environment".

  mysql-test/r/grant2.result
    1.12 05/09/01 16:52:55 dlenev@stripped +9 -0
    Added test for bug #12423 "Deadlock when doing FLUSH PRIVILEGES and GRANT in 
    multi-threaded environment".

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	dlenev
# Host:	brandersnatch.localdomain
# Root:	/home/dlenev/src/mysql-4.1-bg12423

--- 1.588/sql/mysqld.cc	2005-08-20 01:44:27 +04:00
+++ 1.589/sql/mysqld.cc	2005-09-01 16:52:55 +04:00
@@ -3079,7 +3079,7 @@
   */
   error_handler_hook = my_message_sql;
   start_signal_handler();				// Creates pidfile
-  if (acl_init((THD *)0, opt_noacl) || 
+  if (acl_init(opt_noacl) ||
       my_tz_init((THD *)0, default_tz_name, opt_bootstrap))
   {
     abort_loop=1;
@@ -3096,7 +3096,7 @@
     exit(1);
   }
   if (!opt_noacl)
-    (void) grant_init((THD *)0);
+    (void) grant_init();
 
 #ifdef HAVE_DLOPEN
   if (!opt_noacl)

--- 1.166/sql/sql_acl.cc	2005-06-20 21:21:33 +04:00
+++ 1.167/sql/sql_acl.cc	2005-09-01 16:52:55 +04:00
@@ -62,17 +62,20 @@
 static DYNAMIC_ARRAY acl_wild_hosts;
 static hash_filo *acl_cache;
 static uint grant_version=0;
-static uint priv_version=0; /* Version of priv tables. incremented by acl_init */
+static uint priv_version=0; /* Version of priv tables. incremented by acl_load */
 static ulong get_access(TABLE *form,uint fieldnr, uint *next_field=0);
 static int acl_compare(ACL_ACCESS *a,ACL_ACCESS *b);
 static ulong get_sort(uint count,...);
 static void init_check_host(void);
 static ACL_USER *find_acl_user(const char *host, const char *user);
-static bool update_user_table(THD *thd, const char *host, const char *user,
+static bool update_user_table(THD *thd, TABLE *table,
+                              const char *host, const char *user,
 			      const char *new_password, uint new_password_len);
 static void update_hostname(acl_host_and_ip *host, const char *hostname);
 static bool compare_hostname(const acl_host_and_ip *host,const char *hostname,
 			     const char *ip);
+static my_bool acl_load(THD *thd, TABLE_LIST *tables);
+static my_bool grant_load(TABLE_LIST *tables);
 
 /*
   Convert scrambled password to binary form, according to scramble type, 
@@ -117,79 +120,84 @@
 
 
 /*
-  Read grant privileges from the privilege tables in the 'mysql' database.
+  Initialize structures responsible for user/db-level privilege checking and
+  load privilege information for them from tables in the 'mysql' database.
 
   SYNOPSIS
     acl_init()
-    thd				Thread handler
-    dont_read_acl_tables	Set to 1 if run with --skip-grant
+      dont_read_acl_tables  TRUE if we want to skip loading data from
+                            privilege tables and disable privilege checking.
+
+  NOTES
+    This function is mostly responsible for preparatory steps, main work
+    on initialization and grants loading is done in acl_reload().
 
   RETURN VALUES
     0	ok
     1	Could not initialize grant's
 */
 
-
-my_bool acl_init(THD *org_thd, bool dont_read_acl_tables)
+my_bool acl_init(bool dont_read_acl_tables)
 {
   THD  *thd;
-  TABLE_LIST tables[3];
-  TABLE *table;
-  READ_RECORD read_record_info;
-  MYSQL_LOCK *lock;
-  my_bool return_val=1;
-  bool check_no_resolve= specialflag & SPECIAL_NO_RESOLVE;
-  char tmp_name[NAME_LEN+1];
-
+  my_bool return_val;
   DBUG_ENTER("acl_init");
 
-  if (!acl_cache)
-    acl_cache=new hash_filo(ACL_CACHE_SIZE,0,0,
-			    (hash_get_key) acl_entry_get_key,
-			    (hash_free_key) free, system_charset_info);
+  acl_cache= new hash_filo(ACL_CACHE_SIZE, 0, 0,
+                           (hash_get_key) acl_entry_get_key,
+                           (hash_free_key) free, system_charset_info);
   if (dont_read_acl_tables)
   {
     DBUG_RETURN(0); /* purecov: tested */
   }
 
-  priv_version++; /* Privileges updated */
-
   /*
     To be able to run this from boot, we allocate a temporary THD
   */
   if (!(thd=new THD))
     DBUG_RETURN(1); /* purecov: inspected */
   thd->store_globals();
+  /*
+    It is safe to call acl_reload() since acl_* arrays and hashes which
+    will be freed there are global static objects and thus are initialized
+    by zeros at startup.
+  */
+  return_val= acl_reload(thd);
+  delete thd;
+  /* Remember that we don't have a THD */
+  my_pthread_setspecific_ptr(THR_THD,  0);
+  DBUG_RETURN(return_val);
+}
+
+
+/*
+  Initialize structures responsible for user/db-level privilege checking
+  and load information about grants from open privilege tables.
+
+  SYNOPSIS
+    acl_load()
+      thd     Current thread
+      tables  List containing open "mysql.host", "mysql.user" and
+              "mysql.db" tables.
+
+  RETURN VALUES
+    FALSE  Success
+    TRUE   Error
+*/
+
+static my_bool acl_load(THD *thd, TABLE_LIST *tables)
+{
+  TABLE *table;
+  READ_RECORD read_record_info;
+  my_bool return_val= 1;
+  bool check_no_resolve= specialflag & SPECIAL_NO_RESOLVE;
+  char tmp_name[NAME_LEN+1];
+  DBUG_ENTER("acl_load");
+
+  priv_version++; /* Privileges updated */
 
   acl_cache->clear(1);				// Clear locked hostname cache
-  thd->db= my_strdup("mysql",MYF(0));
-  thd->db_length=5;				// Safety
-  bzero((char*) &tables,sizeof(tables));
-  tables[0].alias=tables[0].real_name=(char*) "host";
-  tables[1].alias=tables[1].real_name=(char*) "user";
-  tables[2].alias=tables[2].real_name=(char*) "db";
-  tables[0].next=tables+1;
-  tables[1].next=tables+2;
-  tables[0].lock_type=tables[1].lock_type=tables[2].lock_type=TL_READ;
-  tables[0].db=tables[1].db=tables[2].db=thd->db;
 
-  uint counter;
-  if (open_tables(thd, tables, &counter))
-  {
-    sql_print_error("Fatal error: Can't open privilege tables: %s",
-		    thd->net.last_error);
-    goto end;
-  }
-  TABLE *ptr[3];				// Lock tables for quick update
-  ptr[0]= tables[0].table;
-  ptr[1]= tables[1].table;
-  ptr[2]= tables[2].table;
-  if (! (lock= mysql_lock_tables(thd, ptr, 3, 0)))
-  {
-    sql_print_error("Fatal error: Can't lock privilege tables: %s",
-		    thd->net.last_error);
-    goto end;
-  }
   init_sql_alloc(&mem, ACL_ALLOC_BLOCK_SIZE, 0);
   init_read_record(&read_record_info,thd,table= tables[0].table,NULL,1,0);
   VOID(my_init_dynamic_array(&acl_hosts,sizeof(ACL_HOST),20,50));
@@ -431,21 +439,10 @@
   freeze_size(&acl_dbs);
   init_check_host();
 
-  mysql_unlock_tables(thd, lock);
   initialized=1;
-  thd->version--;				// Force close to free memory
   return_val=0;
 
 end:
-  close_thread_tables(thd);
-  delete thd;
-  if (org_thd)
-    org_thd->store_globals();			/* purecov: inspected */
-  else
-  {
-    /* Remember that we don't have a THD */
-    my_pthread_setspecific_ptr(THR_THD,  0);
-  }
   DBUG_RETURN(return_val);
 }
 
@@ -469,26 +466,60 @@
 
 
 /*
-  Forget current privileges and read new privileges from the privilege tables
+  Forget current user/db-level privileges and read new privileges
+  from the privilege tables.
 
   SYNOPSIS
     acl_reload()
-    thd			Thread handle (can be NULL)
+      thd  Current thread
+
+  NOTE
+    All tables of calling thread which were open and locked by LOCK TABLES
+    statement will be unlocked and closed.
+    This function is also used for initialization of structures responsible
+    for user/db-level privilege checking.
+
+  RETURN VALUE
+    FALSE  Success
+    TRUE   Failure
 */
 
-void acl_reload(THD *thd)
+my_bool acl_reload(THD *thd)
 {
+  TABLE_LIST tables[3];
   DYNAMIC_ARRAY old_acl_hosts,old_acl_users,old_acl_dbs;
   MEM_ROOT old_mem;
   bool old_initialized;
+  my_bool return_val= 1;
   DBUG_ENTER("acl_reload");
 
-  if (thd && thd->locked_tables)
+  if (thd->locked_tables)
   {					// Can't have locked tables here
     thd->lock=thd->locked_tables;
     thd->locked_tables=0;
     close_thread_tables(thd);
   }
+
+  /*
+    To avoid deadlocks we should obtain table locks before
+    obtaining acl_cache->lock mutex.
+  */
+  bzero((char*) tables, sizeof(tables));
+  tables[0].alias=tables[0].real_name=(char*) "host";
+  tables[1].alias=tables[1].real_name=(char*) "user";
+  tables[2].alias=tables[2].real_name=(char*) "db";
+  tables[0].db=tables[1].db=tables[2].db= (char*) "mysql";
+  tables[0].next= tables+1;
+  tables[1].next= tables+2;
+  tables[0].lock_type=tables[1].lock_type=tables[2].lock_type=TL_READ;
+
+  if (simple_open_n_lock_tables(thd, tables))
+  {
+    sql_print_error("Fatal error: Can't open and lock privilege tables: %s",
+		    thd->net.last_error);
+    goto end;
+  }
+
   if ((old_initialized=initialized))
     VOID(pthread_mutex_lock(&acl_cache->lock));
 
@@ -499,7 +530,7 @@
   delete_dynamic(&acl_wild_hosts);
   hash_free(&acl_check_hosts);
 
-  if (acl_init(thd, 0))
+  if ((return_val= acl_load(thd, tables)))
   {					// Error. Revert to old list
     DBUG_PRINT("error",("Reverting to old privileges"));
     acl_free();				/* purecov: inspected */
@@ -518,7 +549,9 @@
   }
   if (old_initialized)
     VOID(pthread_mutex_unlock(&acl_cache->lock));
-  DBUG_VOID_RETURN;
+end:
+  close_thread_tables(thd);
+  DBUG_RETURN(return_val);
 }
 
 
@@ -1229,7 +1262,13 @@
 bool change_password(THD *thd, const char *host, const char *user,
 		     char *new_password)
 {
+  TABLE_LIST tables;
+  TABLE *table;
+  /* Buffer should be extended when password length is extended. */
+  char buff[512];
+  ulong query_length;
   uint new_password_len= strlen(new_password);
+  bool result= 1;
   DBUG_ENTER("change_password");
   DBUG_PRINT("enter",("host: '%s'  user: '%s'  new_password: '%s'",
 		      host,user,new_password));
@@ -1238,42 +1277,71 @@
   if (check_change_password(thd, host, user, new_password, new_password_len))
     DBUG_RETURN(1);
 
+  bzero((char*) &tables, sizeof(tables));
+  tables.alias=tables.real_name= (char*) "user";
+  tables.db= (char*) "mysql";
+
+#ifdef HAVE_REPLICATION
+  /*
+    GRANT and REVOKE are applied the slave in/exclusion rules as they are
+    some kind of updates to the mysql.% tables.
+  */
+  if (thd->slave_thread && table_rules_on)
+  {
+    /*
+      The tables must be marked "updating" so that tables_ok() takes them into
+      account in tests.  It's ok to leave 'updating' set after tables_ok.
+    */
+    tables.updating= 1;
+    /* Thanks to bzero, tables.next==0 */
+    if (!tables_ok(0, &tables))
+      DBUG_RETURN(0);
+  }
+#endif
+
+  if (!(table= open_ltable(thd, &tables, TL_WRITE)))
+    DBUG_RETURN(1);
+
   VOID(pthread_mutex_lock(&acl_cache->lock));
   ACL_USER *acl_user;
   if (!(acl_user= find_acl_user(host, user)))
   {
     VOID(pthread_mutex_unlock(&acl_cache->lock));
     send_error(thd, ER_PASSWORD_NO_MATCH);
-    DBUG_RETURN(1);
+    goto end;
   }
   /* update loaded acl entry: */
   set_user_salt(acl_user, new_password, new_password_len);
 
-  if (update_user_table(thd,
+  if (update_user_table(thd, table,
 			acl_user->host.hostname ? acl_user->host.hostname : "",
 			acl_user->user ? acl_user->user : "",
 			new_password, new_password_len))
   {
     VOID(pthread_mutex_unlock(&acl_cache->lock)); /* purecov: deadcode */
     send_error(thd,0); /* purecov: deadcode */
-    DBUG_RETURN(1); /* purecov: deadcode */
+    goto end;
   }
 
   acl_cache->clear(1);				// Clear locked hostname cache
   VOID(pthread_mutex_unlock(&acl_cache->lock));
-
-  char buff[512]; /* Extend with extended password length*/
-  ulong query_length=
+  result= 0;
+  query_length=
     my_sprintf(buff,
 	       (buff,"SET PASSWORD FOR \"%-.120s\"@\"%-.120s\"=\"%-.120s\"",
 		acl_user->user ? acl_user->user : "",
 		acl_user->host.hostname ? acl_user->host.hostname : "",
 		new_password));
-  thd->clear_error();
   mysql_update_log.write(thd, buff, query_length);
-  Query_log_event qinfo(thd, buff, query_length, 0, FALSE);
-  mysql_bin_log.write(&qinfo);
-  DBUG_RETURN(0);
+  if (mysql_bin_log.is_open())
+  {
+    thd->clear_error();
+    Query_log_event qinfo(thd, buff, query_length, 0, FALSE);
+    mysql_bin_log.write(&qinfo);
+  }
+end:
+  close_thread_tables(thd);
+  DBUG_RETURN(result);
 }
 
 
@@ -1385,43 +1453,28 @@
   return FALSE;
 }
 
+
 /*
-  Update grants in the user and database privilege tables
+  Update record for user in mysql.user privilege table with new password.
+
+  SYNOPSIS
+    update_user_table()
+      thd               Thread handle
+      table             Pointer to TABLE object for open mysql.user table
+      host/user         Hostname/username pair identifying user for which
+                        new password should be set
+      new_password      New password
+      new_password_len  Length of new password
 */
 
-static bool update_user_table(THD *thd, const char *host, const char *user,
+static bool update_user_table(THD *thd, TABLE *table,
+                              const char *host, const char *user,
 			      const char *new_password, uint new_password_len)
 {
-  TABLE_LIST tables;
-  TABLE *table;
-  bool error=1;
+  int error;
   DBUG_ENTER("update_user_table");
   DBUG_PRINT("enter",("user: %s  host: %s",user,host));
 
-  bzero((char*) &tables,sizeof(tables));
-  tables.alias=tables.real_name=(char*) "user";
-  tables.db=(char*) "mysql";
-
-#ifdef HAVE_REPLICATION
-  /*
-    GRANT and REVOKE are applied the slave in/exclusion rules as they are
-    some kind of updates to the mysql.% tables.
-  */
-  if (thd->slave_thread && table_rules_on)
-  {
-    /*
-      The tables must be marked "updating" so that tables_ok() takes them into
-      account in tests.  It's ok to leave 'updating' set after tables_ok.
-    */
-    tables.updating= 1;
-    /* Thanks to bzero, tables.next==0 */
-    if (!tables_ok(0, &tables))
-      DBUG_RETURN(0);
-  }
-#endif
-
-  if (!(table=open_ltable(thd,&tables,TL_WRITE)))
-    DBUG_RETURN(1); /* purecov: deadcode */
   table->field[0]->store(host,(uint) strlen(host), &my_charset_latin1);
   table->field[1]->store(user,(uint) strlen(user), &my_charset_latin1);
 
@@ -1439,13 +1492,9 @@
   if ((error=table->file->update_row(table->record[1],table->record[0])))
   {
     table->file->print_error(error,MYF(0));	/* purecov: deadcode */
-    goto end;					/* purecov: deadcode */
+    DBUG_RETURN(1);
   }
-  error=0;					// Record updated
-
-end:
-  close_thread_tables(thd);
-  DBUG_RETURN(error);
+  DBUG_RETURN(0);
 }
 
 
@@ -2617,18 +2666,59 @@
 }
 
 
-/* Init grant array if possible */
+/*
+  Initialize structures responsible for table/column-level privilege checking
+  and load information for them from tables in the 'mysql' database.
+
+  SYNOPSIS
+    grant_init()
+
+  RETURN VALUES
+    0	ok
+    1	Could not initialize grant's
+*/
 
-my_bool grant_init(THD *org_thd)
+my_bool grant_init()
 {
   THD  *thd;
-  TABLE_LIST tables[2];
-  MYSQL_LOCK *lock;
+  my_bool return_val;
+  DBUG_ENTER("grant_init");
+
+  if (!(thd= new THD))
+    DBUG_RETURN(1);				/* purecov: deadcode */
+  thd->store_globals();
+  return_val=  grant_reload(thd);
+  delete thd;
+  /* Remember that we don't have a THD */
+  my_pthread_setspecific_ptr(THR_THD,  0);
+  DBUG_RETURN(return_val);
+}
+
+
+/*
+  Initialize structures responsible for table/column-level privilege
+  checking and load information about grants from open privilege tables.
+
+  SYNOPSIS
+    grant_load()
+      thd     Current thread
+      tables  List containing open "mysql.tables_priv" and
+              "mysql.columns_priv" tables.
+
+  RETURN VALUES
+    FALSE - success
+    TRUE  - error
+*/
+
+static my_bool grant_load(TABLE_LIST *tables)
+{
   MEM_ROOT *memex_ptr;
   my_bool return_val= 1;
   TABLE *t_table, *c_table;
   bool check_no_resolve= specialflag & SPECIAL_NO_RESOLVE;
-  DBUG_ENTER("grant_init");
+  MEM_ROOT **save_mem_root_ptr= my_pthread_getspecific_ptr(MEM_ROOT**,
+                                                           THR_MALLOC);
+  DBUG_ENTER("grant_load");
 
   grant_option = FALSE;
   (void) hash_init(&column_priv_hash,&my_charset_latin1,
@@ -2636,32 +2726,6 @@
 		   (hash_free_key) free_grant_table,0);
   init_sql_alloc(&memex, ACL_ALLOC_BLOCK_SIZE, 0);
 
-  /* Don't do anything if running with --skip-grant */
-  if (!initialized)
-    DBUG_RETURN(0);				/* purecov: tested */
-
-  if (!(thd=new THD))
-    DBUG_RETURN(1);				/* purecov: deadcode */
-  thd->store_globals();
-  thd->db= my_strdup("mysql",MYF(0));
-  thd->db_length=5;				// Safety
-  bzero((char*) &tables, sizeof(tables));
-  tables[0].alias=tables[0].real_name= (char*) "tables_priv";
-  tables[1].alias=tables[1].real_name= (char*) "columns_priv";
-  tables[0].next=tables+1;
-  tables[0].lock_type=tables[1].lock_type=TL_READ;
-  tables[0].db=tables[1].db=thd->db;
-
-  uint counter;
-  if (open_tables(thd, tables, &counter))
-    goto end;
-
-  TABLE *ptr[2];				// Lock tables for quick update
-  ptr[0]= tables[0].table;
-  ptr[1]= tables[1].table;
-  if (! (lock= mysql_lock_tables(thd, ptr, 2, 0)))
-    goto end;
-
   t_table = tables[0].table; c_table = tables[1].table;
   t_table->file->ha_index_init(0);
   if (t_table->file->index_first(t_table->record[0]))
@@ -2671,7 +2735,6 @@
   }
   grant_option= TRUE;
 
-  /* Will be restored by org_thd->store_globals() */
   memex_ptr= &memex;
   my_pthread_setspecific_ptr(THR_MALLOC, &memex_ptr);
   do
@@ -2708,48 +2771,63 @@
 
 end_unlock:
   t_table->file->ha_index_end();
-  mysql_unlock_tables(thd, lock);
-  thd->version--;				// Force close to free memory
-
-end:
-  close_thread_tables(thd);
-  delete thd;
-  if (org_thd)
-    org_thd->store_globals();
-  else
-  {
-    /* Remember that we don't have a THD */
-    my_pthread_setspecific_ptr(THR_THD,  0);
-  }
+  my_pthread_setspecific_ptr(THR_MALLOC, save_mem_root_ptr);
   DBUG_RETURN(return_val);
 }
 
 
 /*
- Reload grant array (table and column privileges) if possible
+  Reload information about table and column level privileges if possible.
 
   SYNOPSIS
     grant_reload()
-    thd			Thread handler (can be NULL)
+      thd  Current thread
 
   NOTES
-    Locked tables are checked by acl_init and doesn't have to be checked here
+    Locked tables are checked by acl_reload() and doesn't have to be checked
+    in this call.
+    This function is also used for initialization of structures responsible
+    for table/column-level privilege checking.
+
+  RETURN VALUE
+    FALSE Success
+    TRUE  Error
 */
 
-void grant_reload(THD *thd)
+my_bool grant_reload(THD *thd)
 {
+  TABLE_LIST tables[2];
   HASH old_column_priv_hash;
   bool old_grant_option;
   MEM_ROOT old_mem;
+  my_bool return_val= 1;
   DBUG_ENTER("grant_reload");
 
+  /* Don't do anything if running with --skip-grant-tables */
+  if (!initialized)
+    DBUG_RETURN(0);
+
+  bzero((char*) tables, sizeof(tables));
+  tables[0].alias=tables[0].real_name= (char*) "tables_priv";
+  tables[1].alias=tables[1].real_name= (char*) "columns_priv";
+  tables[0].db=tables[1].db= (char *) "mysql";
+  tables[0].next=tables+1;
+  tables[0].lock_type=tables[1].lock_type=TL_READ;
+
+  /*
+    To avoid deadlocks we should obtain table locks before
+    obtaining LOCK_grant rwlock.
+  */
+  if (simple_open_n_lock_tables(thd, tables))
+    goto end;
+
   rw_wrlock(&LOCK_grant);
   grant_version++;
   old_column_priv_hash= column_priv_hash;
   old_grant_option= grant_option;
   old_mem= memex;
 
-  if (grant_init(thd))
+  if ((return_val= grant_load(tables)))
   {						// Error. Revert to old hash
     DBUG_PRINT("error",("Reverting to old privileges"));
     grant_free();				/* purecov: deadcode */
@@ -2763,7 +2841,9 @@
     free_root(&old_mem,MYF(0));
   }
   rw_unlock(&LOCK_grant);
-  DBUG_VOID_RETURN;
+end:
+  close_thread_tables(thd);
+  DBUG_RETURN(return_val);
 }
 
 

--- 1.32/sql/sql_acl.h	2005-01-24 17:48:17 +03:00
+++ 1.33/sql/sql_acl.h	2005-09-01 16:52:55 +04:00
@@ -134,8 +134,8 @@
 /* prototypes */
 
 bool hostname_requires_resolving(const char *hostname);
-my_bool  acl_init(THD *thd, bool dont_read_acl_tables);
-void acl_reload(THD *thd);
+my_bool  acl_init(bool dont_read_acl_tables);
+my_bool acl_reload(THD *thd);
 void acl_free(bool end=0);
 ulong acl_get(const char *host, const char *ip,
 	      const char *user, const char *db, my_bool db_is_pattern);
@@ -151,9 +151,9 @@
 int mysql_table_grant(THD *thd, TABLE_LIST *table, List <LEX_USER> &user_list,
 		      List <LEX_COLUMN> &column_list, ulong rights,
 		      bool revoke);
-my_bool grant_init(THD *thd);
+my_bool grant_init();
 void grant_free(void);
-void grant_reload(THD *thd);
+my_bool grant_reload(THD *thd);
 bool check_grant(THD *thd, ulong want_access, TABLE_LIST *tables,
 		 uint show_command, uint number, bool dont_print_error);
 bool check_grant_column (THD *thd,TABLE *table, const char *name, uint length,

--- 1.460/sql/sql_parse.cc	2005-08-15 19:36:16 +04:00
+++ 1.461/sql/sql_parse.cc	2005-09-01 16:52:55 +04:00
@@ -4971,10 +4971,27 @@
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
   if (options & REFRESH_GRANT)
   {
-    acl_reload(thd);
-    grant_reload(thd);
-    if (mqh_used)
-      reset_mqh(thd,(LEX_USER *) NULL,TRUE);
+    THD *tmp_thd= 0;
+    /*
+      If reload_acl_and_cache() is called from SIGHUP handler we have to
+      allocate temporary THD for execution of acl_reload()/grant_reload().
+    */
+    if (!thd && (thd= (tmp_thd= new THD)))
+      thd->store_globals();
+    if (thd)
+    {
+      (void)acl_reload(thd);
+      (void)grant_reload(thd);
+      if (mqh_used)
+        reset_mqh(thd, (LEX_USER *) NULL, TRUE);
+    }
+    if (tmp_thd)
+    {
+      delete tmp_thd;
+      /* Remember that we don't have a THD */
+      my_pthread_setspecific_ptr(THR_THD,  0);
+      thd= 0;
+    }
   }
 #endif
   if (options & REFRESH_LOG)

--- 1.11/mysql-test/r/grant2.result	2005-03-27 17:46:00 +04:00
+++ 1.12/mysql-test/r/grant2.result	2005-09-01 16:52:55 +04:00
@@ -96,3 +96,12 @@
 REVOKE ALL ON mysqltest_1.t1 FROM mysqltest_1@'127.0.0.0/255.0.0.0';
 drop table mysqltest_1.t1;
 drop database mysqltest_1;
+lock table mysql.user write;
+ flush privileges;
+ grant all on *.* to 'mysqltest_1'@'localhost';
+unlock tables;
+lock table mysql.user write;
+ set password for 'mysqltest_1'@'localhost' = password('');
+ revoke all on *.* from 'mysqltest_1'@'localhost';
+unlock tables;
+drop user 'mysqltest_1'@'localhost';

--- 1.12/mysql-test/t/grant2.test	2005-07-28 04:21:42 +04:00
+++ 1.13/mysql-test/t/grant2.test	2005-09-01 16:52:55 +04:00
@@ -125,4 +125,47 @@
 drop table mysqltest_1.t1;
 drop database mysqltest_1;
 
+
+# Bug #12423 "Deadlock when doing FLUSH PRIVILEGES and GRANT in 
+# multi-threaded environment". We should be able to execute FLUSH
+# PRIVILEGES and SET PASSWORD simultaneously with other account
+# management commands (such as GRANT and REVOKE) without causing
+# deadlocks. To achieve this we should ensure that all account
+# management commands take table and internal locks in the same order.
+connect (con2root,localhost,root,,);
+connect (con3root,localhost,root,,);
+# Check that we can execute FLUSH PRIVILEGES and GRANT simultaneously
+# This will check that locks are taken in proper order during both
+# user/db-level and table/column-level privileges reloading.
+connection default;
+lock table mysql.user write;
+connection con2root;
+send flush privileges;
+connection con3root;
+send grant all on *.* to 'mysqltest_1'@'localhost';
+connection default;
+unlock tables;
+connection con2root;
+reap;
+connection con3root;
+reap;
+# Check for simultaneous SET PASSWORD and REVOKE.
+connection default;
+lock table mysql.user write;
+connection con2root;
+send set password for 'mysqltest_1'@'localhost' = password('');
+connection con3root;
+send revoke all on *.* from 'mysqltest_1'@'localhost';
+connection default;
+unlock tables;
+connection con2root;
+reap;
+connection con3root;
+reap;
+connection default;
+# Clean-up
+drop user 'mysqltest_1'@'localhost';
+disconnect con2root;
+disconnect con3root;
+
 # End of 4.1 tests