Commit 6e265308 authored by unknown's avatar unknown

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.


mysql-test/r/grant2.result:
  Added test for bug #12423 "Deadlock when doing FLUSH PRIVILEGES and GRANT in 
  multi-threaded environment".
mysql-test/t/grant2.test:
  Added test for bug #12423 "Deadlock when doing FLUSH PRIVILEGES and GRANT in 
  multi-threaded environment".
sql/mysqld.cc:
  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.
sql/sql_acl.cc:
  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/sql_acl.h:
  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_parse.cc:
  If reload_acl_and_cache() is called from SIGHUP handler we have to
  allocate temporary THD for execution of acl_reload()/grant_reload().
parent 27f0e6ac
...@@ -96,3 +96,12 @@ i ...@@ -96,3 +96,12 @@ i
REVOKE ALL ON mysqltest_1.t1 FROM mysqltest_1@'127.0.0.0/255.0.0.0'; REVOKE ALL ON mysqltest_1.t1 FROM mysqltest_1@'127.0.0.0/255.0.0.0';
drop table mysqltest_1.t1; drop table mysqltest_1.t1;
drop database mysqltest_1; 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';
...@@ -125,4 +125,47 @@ REVOKE ALL ON mysqltest_1.t1 FROM mysqltest_1@'127.0.0.0/255.0.0.0'; ...@@ -125,4 +125,47 @@ REVOKE ALL ON mysqltest_1.t1 FROM mysqltest_1@'127.0.0.0/255.0.0.0';
drop table mysqltest_1.t1; drop table mysqltest_1.t1;
drop database mysqltest_1; 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 # End of 4.1 tests
...@@ -3079,7 +3079,7 @@ we force server id to 2, but this MySQL server will not act as a slave."); ...@@ -3079,7 +3079,7 @@ we force server id to 2, but this MySQL server will not act as a slave.");
*/ */
error_handler_hook = my_message_sql; error_handler_hook = my_message_sql;
start_signal_handler(); // Creates pidfile 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)) my_tz_init((THD *)0, default_tz_name, opt_bootstrap))
{ {
abort_loop=1; abort_loop=1;
...@@ -3096,7 +3096,7 @@ we force server id to 2, but this MySQL server will not act as a slave."); ...@@ -3096,7 +3096,7 @@ we force server id to 2, but this MySQL server will not act as a slave.");
exit(1); exit(1);
} }
if (!opt_noacl) if (!opt_noacl)
(void) grant_init((THD *)0); (void) grant_init();
#ifdef HAVE_DLOPEN #ifdef HAVE_DLOPEN
if (!opt_noacl) if (!opt_noacl)
......
This diff is collapsed.
...@@ -134,8 +134,8 @@ class ACL_DB :public ACL_ACCESS ...@@ -134,8 +134,8 @@ class ACL_DB :public ACL_ACCESS
/* prototypes */ /* prototypes */
bool hostname_requires_resolving(const char *hostname); bool hostname_requires_resolving(const char *hostname);
my_bool acl_init(THD *thd, bool dont_read_acl_tables); my_bool acl_init(bool dont_read_acl_tables);
void acl_reload(THD *thd); my_bool acl_reload(THD *thd);
void acl_free(bool end=0); void acl_free(bool end=0);
ulong acl_get(const char *host, const char *ip, ulong acl_get(const char *host, const char *ip,
const char *user, const char *db, my_bool db_is_pattern); const char *user, const char *db, my_bool db_is_pattern);
...@@ -151,9 +151,9 @@ int mysql_grant(THD *thd, const char *db, List <LEX_USER> &user_list, ...@@ -151,9 +151,9 @@ int mysql_grant(THD *thd, const char *db, List <LEX_USER> &user_list,
int mysql_table_grant(THD *thd, TABLE_LIST *table, List <LEX_USER> &user_list, int mysql_table_grant(THD *thd, TABLE_LIST *table, List <LEX_USER> &user_list,
List <LEX_COLUMN> &column_list, ulong rights, List <LEX_COLUMN> &column_list, ulong rights,
bool revoke); bool revoke);
my_bool grant_init(THD *thd); my_bool grant_init();
void grant_free(void); 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, bool check_grant(THD *thd, ulong want_access, TABLE_LIST *tables,
uint show_command, uint number, bool dont_print_error); uint show_command, uint number, bool dont_print_error);
bool check_grant_column (THD *thd,TABLE *table, const char *name, uint length, bool check_grant_column (THD *thd,TABLE *table, const char *name, uint length,
......
...@@ -4971,10 +4971,27 @@ bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables, ...@@ -4971,10 +4971,27 @@ bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables,
#ifndef NO_EMBEDDED_ACCESS_CHECKS #ifndef NO_EMBEDDED_ACCESS_CHECKS
if (options & REFRESH_GRANT) if (options & REFRESH_GRANT)
{ {
acl_reload(thd); THD *tmp_thd= 0;
grant_reload(thd); /*
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) if (mqh_used)
reset_mqh(thd,(LEX_USER *) NULL,TRUE); 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 #endif
if (options & REFRESH_LOG) if (options & REFRESH_LOG)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment