bk commit into 4.0 tree (ingo:1.2093) BUG#7823

[ingo]

Below is the list of changes that have just been committed into a local
4.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.2093 05/04/27 12:40:37 ingo@stripped +3 -0
  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/sql_insert.cc
    1.117 05/04/27 12:40:35 ingo@stripped +55 -20
    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.

  sql/mysql_priv.h
    1.232 05/04/27 12:40:35 ingo@stripped +4 -1
    Bug#7823 - FLUSH TABLES WITH READ LOCK + INSERT DELAYED = deadlock
    Changed existing and added new function declarations.

  sql/lock.cc
    1.50 05/04/27 12:40:35 ingo@stripped +49 -2
    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.

# 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.0-bug7823

--- 1.49/sql/lock.cc	Tue Nov 30 22:20:49 2004
+++ 1.50/sql/lock.cc	Wed Apr 27 12:40:35 2005
@@ -79,7 +79,8 @@
 static void print_lock_error(int error);
 
 
-MYSQL_LOCK *mysql_lock_tables(THD *thd,TABLE **tables,uint count)
+MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **tables, uint count,
+                              bool ignore_global_read_lock)
 {
   MYSQL_LOCK *sql_lock;
   TABLE *write_lock_used;
@@ -90,7 +91,7 @@
     if (!(sql_lock = get_lock_data(thd,tables,count, 0,&write_lock_used)))
       break;
 
-    if (global_read_lock && write_lock_used)
+    if (global_read_lock && write_lock_used && ! ignore_global_read_lock)
     {
       /*
 	Someone has issued LOCK ALL TABLES FOR READ and we want a write lock
@@ -865,3 +866,49 @@
   pthread_mutex_unlock(&LOCK_open);
   thd->global_read_lock= MADE_GLOBAL_READ_LOCK_BLOCK_COMMIT;
 }
+
+
+/*
+  Set protection against global read lock.
+
+  SYNOPSIS
+    set_protect_against_global_read_lock()
+    void
+
+  RETURN
+    FALSE       OK, no global read lock exists.
+    TRUE        Error, global read lock exists already.
+*/
+
+my_bool set_protect_against_global_read_lock(void)
+{
+  my_bool       global_read_lock_exists;
+
+  pthread_mutex_lock(&LOCK_open);
+  if (! (global_read_lock_exists= test(global_read_lock)))
+    protect_against_global_read_lock++;
+  pthread_mutex_unlock(&LOCK_open);
+  return global_read_lock_exists;
+}
+
+
+/*
+  Unset protection against global read lock.
+
+  SYNOPSIS
+    unset_protect_against_global_read_lock()
+    void
+
+  RETURN
+    void
+*/
+
+void unset_protect_against_global_read_lock(void)
+{
+  pthread_mutex_lock(&LOCK_open);
+  protect_against_global_read_lock--;
+  pthread_mutex_unlock(&LOCK_open);
+  pthread_cond_broadcast(&COND_refresh);
+}
+
+

--- 1.231/sql/mysql_priv.h	Thu Mar  3 19:51:26 2005
+++ 1.232/sql/mysql_priv.h	Wed Apr 27 12:40:35 2005
@@ -766,7 +766,8 @@
 extern struct st_VioSSLAcceptorFd * ssl_acceptor_fd;
 #endif /* HAVE_OPENSSL */
 
-MYSQL_LOCK *mysql_lock_tables(THD *thd,TABLE **table,uint count);
+MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **table, uint count,
+                              bool ignore_global_read_lock= FALSE);
 void mysql_unlock_tables(THD *thd, MYSQL_LOCK *sql_lock);
 void mysql_unlock_read_tables(THD *thd, MYSQL_LOCK *sql_lock);
 void mysql_unlock_some_tables(THD *thd, TABLE **table,uint count);
@@ -779,6 +780,8 @@
 bool wait_if_global_read_lock(THD *thd, bool abort_on_refresh, bool is_not_commit);
 void start_waiting_global_read_lock(THD *thd);
 void make_global_read_lock_block_commit(THD *thd);
+my_bool set_protect_against_global_read_lock(void);
+void unset_protect_against_global_read_lock(void);
 
 /* Lock based on name */
 int lock_and_wait_for_table_name(THD *thd, TABLE_LIST *table_list);

--- 1.116/sql/sql_insert.cc	Thu Dec  2 23:59:45 2004
+++ 1.117/sql/sql_insert.cc	Wed Apr 27 12:40:35 2005
@@ -647,27 +647,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=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 @@
 	  !(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 @@
 	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 @@
 	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 @@
 	  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 @@
   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 @@
 
     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
       }