MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:ingo Date:May 15 2006 1:46pm
Subject:bk commit into 5.0 tree (ingo:1.2094) BUG#19815
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of mydev. When mydev 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.2094 06/05/15 15:46:43 ingo@stripped +3 -0
  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
    1.128 06/05/15 15:46:38 ingo@stripped +46 -14
    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().

  mysql-test/t/lock_multi.test
    1.14 06/05/15 15:46:38 ingo@stripped +35 -0
    Bug#19815 - CREATE/RENAME/DROP DATABASE can deadlock on a global read lock
    The test case

  mysql-test/r/lock_multi.result
    1.16 06/05/15 15:46:38 ingo@stripped +8 -0
    Bug#19815 - CREATE/RENAME/DROP DATABASE can deadlock on a global read lock
    The test result

# 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:	ingo
# Host:	chilla.local
# Root:	/home/mydev/mysql-5.0-aid

--- 1.127/sql/sql_db.cc	2006-03-01 15:43:51 +01:00
+++ 1.128/sql/sql_db.cc	2006-05-15 15:46:38 +02:00
@@ -424,16 +424,27 @@
     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 @@
   }
 
 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 @@
   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 @@
   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 @@
   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 @@
 
 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 @@
     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);
 }
 

--- 1.15/mysql-test/r/lock_multi.result	2005-09-15 21:17:37 +02:00
+++ 1.16/mysql-test/r/lock_multi.result	2006-05-15 15:46:38 +02:00
@@ -43,3 +43,11 @@
 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

--- 1.13/mysql-test/t/lock_multi.test	2005-09-15 21:17:37 +02:00
+++ 1.14/mysql-test/t/lock_multi.test	2006-05-15 15:46:38 +02:00
@@ -107,3 +107,38 @@
 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;
+
Thread
bk commit into 5.0 tree (ingo:1.2094) BUG#19815ingo15 May