From d00441e328bd126d43fe88d157d6140013d17c90 Mon Sep 17 00:00:00 2001 From: "ingo@mysql.com" <> Date: Mon, 29 May 2006 15:26:23 +0200 Subject: Bug#19815 - CREATE/RENAME/DROP DATABASE can deadlock on a global read lock The order of acquiring LOCK_mysql_create_db and wait_if_global_read_lock() was wrong. It could happen that a thread held LOCK_mysql_create_db while waiting for the global read lock to be released. The thread with the global read lock could try to administrate a database too. It would first try to lock LOCK_mysql_create_db and hang... The check if the current thread has the global read lock is done in wait_if_global_read_lock(), which could not be reached because of the hang in LOCK_mysql_create_db. Now I exchanged the order of acquiring LOCK_mysql_create_db and wait_if_global_read_lock(). This makes wait_if_global_read_lock() fail with an error message for the thread with the global read lock. No deadlock happens. --- sql/sql_db.cc | 60 +++++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 46 insertions(+), 14 deletions(-) (limited to 'sql/sql_db.cc') diff --git a/sql/sql_db.cc b/sql/sql_db.cc index 4caa0076c60..a52972753a7 100644 --- a/sql/sql_db.cc +++ b/sql/sql_db.cc @@ -424,16 +424,27 @@ bool mysql_create_db(THD *thd, char *db, HA_CREATE_INFO *create_info, my_error(ER_DB_CREATE_EXISTS, MYF(0), db); DBUG_RETURN(-1); } - - VOID(pthread_mutex_lock(&LOCK_mysql_create_db)); - /* do not create database if another thread is holding read lock */ + /* + Do not create database if another thread is holding read lock. + Wait for global read lock before acquiring LOCK_mysql_create_db. + After wait_if_global_read_lock() we have protection against another + global read lock. If we would acquire LOCK_mysql_create_db first, + another thread could step in and get the global read lock before we + reach wait_if_global_read_lock(). If this thread tries the same as we + (admin a db), it would then go and wait on LOCK_mysql_create_db... + Furthermore wait_if_global_read_lock() checks if the current thread + has the global read lock and refuses the operation with + ER_CANT_UPDATE_WITH_READLOCK if applicable. + */ if (wait_if_global_read_lock(thd, 0, 1)) { error= -1; goto exit2; } + VOID(pthread_mutex_lock(&LOCK_mysql_create_db)); + /* Check directory */ strxmov(path, mysql_data_home, "/", db, NullS); path_len= unpack_dirname(path,path); // Convert if not unix @@ -537,9 +548,9 @@ bool mysql_create_db(THD *thd, char *db, HA_CREATE_INFO *create_info, } exit: + VOID(pthread_mutex_unlock(&LOCK_mysql_create_db)); start_waiting_global_read_lock(thd); exit2: - VOID(pthread_mutex_unlock(&LOCK_mysql_create_db)); DBUG_RETURN(error); } @@ -553,12 +564,23 @@ bool mysql_alter_db(THD *thd, const char *db, HA_CREATE_INFO *create_info) int error= 0; DBUG_ENTER("mysql_alter_db"); - VOID(pthread_mutex_lock(&LOCK_mysql_create_db)); - - /* do not alter database if another thread is holding read lock */ + /* + Do not alter database if another thread is holding read lock. + Wait for global read lock before acquiring LOCK_mysql_create_db. + After wait_if_global_read_lock() we have protection against another + global read lock. If we would acquire LOCK_mysql_create_db first, + another thread could step in and get the global read lock before we + reach wait_if_global_read_lock(). If this thread tries the same as we + (admin a db), it would then go and wait on LOCK_mysql_create_db... + Furthermore wait_if_global_read_lock() checks if the current thread + has the global read lock and refuses the operation with + ER_CANT_UPDATE_WITH_READLOCK if applicable. + */ if ((error=wait_if_global_read_lock(thd,0,1))) goto exit2; + VOID(pthread_mutex_lock(&LOCK_mysql_create_db)); + /* Check directory */ strxmov(path, mysql_data_home, "/", db, "/", MY_DB_OPT_FILE, NullS); fn_format(path, path, "", "", MYF(MY_UNPACK_FILENAME)); @@ -596,9 +618,9 @@ bool mysql_alter_db(THD *thd, const char *db, HA_CREATE_INFO *create_info) send_ok(thd, result); exit: + VOID(pthread_mutex_unlock(&LOCK_mysql_create_db)); start_waiting_global_read_lock(thd); exit2: - VOID(pthread_mutex_unlock(&LOCK_mysql_create_db)); DBUG_RETURN(error); } @@ -630,15 +652,26 @@ bool mysql_rm_db(THD *thd,char *db,bool if_exists, bool silent) TABLE_LIST* dropped_tables= 0; DBUG_ENTER("mysql_rm_db"); - VOID(pthread_mutex_lock(&LOCK_mysql_create_db)); - - /* do not drop database if another thread is holding read lock */ + /* + Do not drop database if another thread is holding read lock. + Wait for global read lock before acquiring LOCK_mysql_create_db. + After wait_if_global_read_lock() we have protection against another + global read lock. If we would acquire LOCK_mysql_create_db first, + another thread could step in and get the global read lock before we + reach wait_if_global_read_lock(). If this thread tries the same as we + (admin a db), it would then go and wait on LOCK_mysql_create_db... + Furthermore wait_if_global_read_lock() checks if the current thread + has the global read lock and refuses the operation with + ER_CANT_UPDATE_WITH_READLOCK if applicable. + */ if (wait_if_global_read_lock(thd, 0, 1)) { error= -1; goto exit2; } + VOID(pthread_mutex_lock(&LOCK_mysql_create_db)); + (void) sprintf(path,"%s/%s",mysql_data_home,db); length= unpack_dirname(path,path); // Convert if not unix strmov(path+length, MY_DB_OPT_FILE); // Append db option file name @@ -747,7 +780,6 @@ bool mysql_rm_db(THD *thd,char *db,bool if_exists, bool silent) exit: (void)sp_drop_db_routines(thd, db); /* QQ Ignore errors for now */ - start_waiting_global_read_lock(thd); /* If this database was the client's selected database, we silently change the client's selected database to nothing (to have an empty SELECT DATABASE() @@ -776,9 +808,9 @@ exit: thd->db= 0; thd->db_length= 0; } -exit2: VOID(pthread_mutex_unlock(&LOCK_mysql_create_db)); - + start_waiting_global_read_lock(thd); +exit2: DBUG_RETURN(error); } -- cgit v1.2.1