Commit 4f8407b6 authored by unknown's avatar unknown
Browse files

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.


mysql-test/r/lock_multi.result:
  Bug#19815 - CREATE/RENAME/DROP DATABASE can deadlock on a global read lock
  The test result
mysql-test/t/lock_multi.test:
  Bug#19815 - CREATE/RENAME/DROP DATABASE can deadlock on a global read lock
  The test case
sql/sql_db.cc:
  Bug#19815 - CREATE/RENAME/DROP DATABASE can deadlock on a global read lock
  Exchanged the order of acquiring LOCK_mysql_create_db
  and wait_if_global_read_lock().
parent 0086d5ab

mysql-test/r/lock_multi.result

+8 −0
Original line number Diff line number Diff line
@@ -43,3 +43,11 @@ Field Type Null Key Default Extra 
a	int(11)	YES		NULL	

unlock tables;

drop table t1;

CREATE DATABASE mysqltest_1;

FLUSH TABLES WITH READ LOCK;

 DROP DATABASE mysqltest_1;

DROP DATABASE mysqltest_1;

ERROR HY000: Can't execute the query because you have a conflicting read lock

UNLOCK TABLES;

DROP DATABASE mysqltest_1;

ERROR HY000: Can't drop database 'mysqltest_1'; database doesn't exist

mysql-test/t/lock_multi.test

+35 −0
Original line number Diff line number Diff line
@@ -107,3 +107,38 @@ show columns from t1; 
connection locker;

unlock tables;

drop table t1;



#

# Bug#19815 - CREATE/RENAME/DROP DATABASE can deadlock on a global read lock

#

connect (con1,localhost,root,,);

connect (con2,localhost,root,,);

#

connection con1;

CREATE DATABASE mysqltest_1;

FLUSH TABLES WITH READ LOCK;

#

# With bug in place: acquire LOCK_mysql_create_table and

# wait in wait_if_global_read_lock().

connection con2;

send DROP DATABASE mysqltest_1;

--sleep 1

#

# With bug in place: try to acquire LOCK_mysql_create_table...

# When fixed: Reject dropping db because of the read lock.

connection con1;

--error ER_CANT_UPDATE_WITH_READLOCK

DROP DATABASE mysqltest_1;

UNLOCK TABLES;

#

connection con2;

reap;

#

connection default;

disconnect con1;

disconnect con2;

# This must have been dropped by connection 2 already,

# which waited until the global read lock was released.

--error ER_DB_DROP_EXISTS

DROP DATABASE mysqltest_1;

sql/sql_db.cc

+46 −14
Original line number Diff line number Diff line
@@ -425,15 +425,26 @@ bool mysql_create_db(THD *thd, char *db, HA_CREATE_INFO *create_info, 
    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 @@ bool mysql_rm_db(THD *thd,char *db,bool if_exists, bool silent) 
    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);

}