bk commit into 4.1 tree (ingo:1.2213) BUG#7823

[ingo]

Below is the list of changes that have just been committed into a local
4.1 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.2213 05/04/27 20:54:19 ingo@stripped +1 -0
  Bug#7823 - FLUSH TABLES WITH READ LOCK + INSERT DELAYED = deadlock
  After merge fix.

  sql/sql_insert.cc
    1.160 05/04/27 20:54:15 ingo@stripped +55 -20
    Bug#7823 - FLUSH TABLES WITH READ LOCK + INSERT DELAYED = deadlock
    After merge fix.

# 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-4.1-4100

--- 1.159/sql/sql_insert.cc	Wed Apr 27 18:59:27 2005
+++ 1.160/sql/sql_insert.cc	Wed Apr 27 20:54:15 2005
@@ -851,27 +851,42 @@
 {
   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();
 	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++;
@@ -880,10 +895,8 @@
 	  !(tmp->thd.query=my_strdup(table_list->real_name,MYF(MY_WME))))
       {
 	delete tmp;
-	thd->fatal_error();
 	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;
@@ -899,10 +912,8 @@
 	pthread_mutex_unlock(&tmp->mutex);
 	tmp->unlock();
 	delete tmp;
-	thd->fatal_error();
-	pthread_mutex_unlock(&LOCK_delayed_create);
 	net_printf(thd,ER_CANT_CREATE_THREAD,error);
-	DBUG_RETURN(0);
+	goto err1;
       }
 
       /* Wait until table is open */
@@ -912,6 +923,7 @@
 	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)
       {
@@ -923,28 +935,34 @@
 	  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.is_fatal_error)
     thd->fatal_error();
+  /* Unlock the delayed insert object after its last access. */
+  tmp->unlock();
   DBUG_RETURN((table_list->table=table));
+
+ err1:
+  thd->fatal_error();
+  unset_protect_against_global_read_lock();
+ err:
+  pthread_mutex_unlock(&LOCK_delayed_create);
+  DBUG_RETURN(0); // Continue with normal insert
 }
 
 
@@ -1165,6 +1183,14 @@
   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())
@@ -1279,8 +1305,17 @@
 
     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= 1;			// Some fatal error
         thd->killed= 1;