Bug#7823 - FLUSH TABLES WITH READ LOCK + INSERT DELAYED = deadlock

Added protection against global read lock while creating and
initializing a delayed insert handler.
Allowed to ignore a global read lock when locking the table
inside the delayed insert handler.
Added some minor improvements.


sql/lock.cc:
  Bug#7823 - FLUSH TABLES WITH READ LOCK + INSERT DELAYED = deadlock
  Changed mysql_lock_tables() to allow for ignoring global read lock.
  Added functions to set/unset protection against global read lock.
sql/mysql_priv.h:
  Bug#7823 - FLUSH TABLES WITH READ LOCK + INSERT DELAYED = deadlock
  Changed existing and added new function declarations.
sql/sql_insert.cc:
  Bug#7823 - FLUSH TABLES WITH READ LOCK + INSERT DELAYED = deadlock
  Added and extended some comments.
  Added a protection against global read lock while a handler is
  created and initialized.
  Moved the unlock of the delayed insert object past its last usage
  in delayed_get_table().
  Changed the table locking in handle_delayed_insert() so that it
  does not wait for global read lock.

1 files changed, 55 insertions, 20 deletions

diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
index 0c62a9af7ba..c6b7b1d6c15 100644
--- a/sql/sql_insert.cc
+++ b/sql/sql_insert.cc
@@ -647,27 +647,42 @@ static TABLE *delayed_get_table(THD *thd,TABLE_LIST *table_list)
 {
   int error;
   delayed_insert *tmp;
+  TABLE *table;
   DBUG_ENTER("delayed_get_table");
 
   if (!table_list->db)
     table_list->db=thd->db;
 
-  /* no match; create a new thread to handle the table */
+  /* Find the thread which handles this table. */
   if (!(tmp=find_handler(thd,table_list)))
   {
-    /* Don't create more than max_insert_delayed_threads */
+    /*
+      No match. Create a new thread to handle the table, but
+      no more than max_insert_delayed_threads.
+    */
     if (delayed_insert_threads >= thd->variables.max_insert_delayed_threads)
       DBUG_RETURN(0);
     thd->proc_info="Creating delayed handler";
     pthread_mutex_lock(&LOCK_delayed_create);
-    if (!(tmp=find_handler(thd,table_list)))	// Was just created
+    /*
+      The first search above was done without LOCK_delayed_create.
+      Another thread might have created the handler in between. Search again.
+    */
+    if (! (tmp= find_handler(thd, table_list)))
     {
+      /*
+        Avoid that a global read lock steps in while we are creating the
+        new thread. It would block trying to open the table. Hence, the
+        DI thread and this thread would wait until after the global
+        readlock is gone. If the read lock exists already, we leave with
+        no table and then switch to non-delayed insert.
+      */
+      if (set_protect_against_global_read_lock())
+        goto err;
       if (!(tmp=new delayed_insert()))
       {
-	thd->fatal_error=1;
 	my_error(ER_OUTOFMEMORY,MYF(0),sizeof(delayed_insert));
-	pthread_mutex_unlock(&LOCK_delayed_create);
-	DBUG_RETURN(0);
+        goto err1;
       }
       pthread_mutex_lock(&LOCK_thread_count);
       thread_count++;
@@ -676,10 +691,8 @@ static TABLE *delayed_get_table(THD *thd,TABLE_LIST *table_list)
 	  !(tmp->thd.query=my_strdup(table_list->real_name,MYF(MY_WME))))
       {
 	delete tmp;
-	thd->fatal_error=1;
 	my_error(ER_OUT_OF_RESOURCES,MYF(0));
-	pthread_mutex_unlock(&LOCK_delayed_create);
-	DBUG_RETURN(0);
+        goto err1;
       }
       tmp->table_list= *table_list;			// Needed to open table
       tmp->table_list.db= tmp->thd.db;
@@ -695,10 +708,8 @@ static TABLE *delayed_get_table(THD *thd,TABLE_LIST *table_list)
 	pthread_mutex_unlock(&tmp->mutex);
 	tmp->unlock();
 	delete tmp;
-	thd->fatal_error=1;
-	pthread_mutex_unlock(&LOCK_delayed_create);
 	net_printf(&thd->net,ER_CANT_CREATE_THREAD,error);
-	DBUG_RETURN(0);
+        goto err1;
       }
 
       /* Wait until table is open */
@@ -708,6 +719,7 @@ static TABLE *delayed_get_table(THD *thd,TABLE_LIST *table_list)
 	pthread_cond_wait(&tmp->cond_client,&tmp->mutex);
       }
       pthread_mutex_unlock(&tmp->mutex);
+      unset_protect_against_global_read_lock();
       thd->proc_info="got old table";
       if (tmp->thd.killed)
       {
@@ -719,28 +731,34 @@ static TABLE *delayed_get_table(THD *thd,TABLE_LIST *table_list)
 	  thd->net.last_errno=tmp->thd.net.last_errno;
 	}
 	tmp->unlock();
-	pthread_mutex_unlock(&LOCK_delayed_create);
-	DBUG_RETURN(0);				// Continue with normal insert
+	goto err;
       }
       if (thd->killed)
       {
 	tmp->unlock();
-	pthread_mutex_unlock(&LOCK_delayed_create);
-	DBUG_RETURN(0);
+	goto err;
       }
     }
     pthread_mutex_unlock(&LOCK_delayed_create);
   }
 
   pthread_mutex_lock(&tmp->mutex);
-  TABLE *table=tmp->get_local_table(thd);
+  table= tmp->get_local_table(thd);
   pthread_mutex_unlock(&tmp->mutex);
-  tmp->unlock();
   if (table)
     thd->di=tmp;
   else if (tmp->thd.fatal_error)
     thd->fatal_error=1;
+  /* Unlock the delayed insert object after its last access. */
+  tmp->unlock();
   DBUG_RETURN((table_list->table=table));
+
+ err1:
+  thd->fatal_error= 1;
+  unset_protect_against_global_read_lock();
+ err:
+  pthread_mutex_unlock(&LOCK_delayed_create);
+  DBUG_RETURN(0); // Continue with normal insert
 }
 
 
@@ -955,6 +973,14 @@ extern "C" pthread_handler_decl(handle_delayed_insert,arg)
   thd->killed=abort_loop;
   pthread_mutex_unlock(&LOCK_thread_count);
 
+  /*
+    Wait until the client runs into pthread_cond_wait(),
+    where we free it after the table is opened and di linked in the list.
+    If we did not wait here, the client might detect the opened table
+    before it is linked to the list. It would release LOCK_delayed_create
+    and allow another thread to create another handler for the same table,
+    since it does not find one in the list.
+  */
   pthread_mutex_lock(&di->mutex);
 #if !defined( __WIN__) && !defined(OS2)	/* Win32 calls this in pthread_create */
   if (my_thread_init())
@@ -1069,8 +1095,17 @@ extern "C" pthread_handler_decl(handle_delayed_insert,arg)
 
     if (di->tables_in_use && ! thd->lock)
     {
-      /* request for new delayed insert */
-      if (!(thd->lock=mysql_lock_tables(thd,&di->table,1)))
+      /*
+        Request for new delayed insert.
+        Lock the table, but avoid to be blocked by a global read lock.
+        If we got here while a global read lock exists, then one or more
+        inserts started before the lock was requested. These are allowed
+        to complete their work before the server returns control to the
+        client which requested the global read lock. The delayed insert
+        handler will close the table and finish when the outstanding
+        inserts are done.
+      */
+      if (! (thd->lock= mysql_lock_tables(thd, &di->table, 1, TRUE)))
       {
 	di->dead=thd->killed=1;			// Fatal error
       }