bzr commit into mysql-6.0-codebase-bugfixing branch (jon.hauglid:3714) Bug#48724

[Jon Olav Hauglid]

#At file:///export/home/z/mysql-6.0-codebase-bugfixing-bug48724/ based on revid:jon.hauglid@stripped

 3714 Jon Olav Hauglid	2009-11-20
      Bug #48724 Deadlock between INSERT DELAYED and FLUSH TABLES
      
      The problem was that if the handler thread was killed at the wrong moment
      (using e.g. FLUSH TABLES), this would not be properly noticed by the
      INSERT DELAYED connection thread. It would be stuck waiting for the handler
      thread to lock its table, while the handler thread would be looping, trying
      to get the connection thread to notice the error.
      
      The root of the problem was that insert delayed had an extra variable "dead"
      used to indicate if the handler thread had been killed. This in addition to
      the usual "thd->killed". Most places both were set, but some only set
      "thd->killed". And Delayed_insert::get_local_table() only checked "dead"
      while waiting for the table to be locked.
      
      This patch removes the "dead" variable and replaces its usage with
      "thd->killed", thereby resolving the issue.

    modified:
      sql/sql_insert.cc
=== modified file 'sql/sql_insert.cc'
--- a/sql/sql_insert.cc	2009-11-17 13:53:32 +0000
+++ b/sql/sql_insert.cc	2009-11-20 10:31:35 +0000
@@ -1776,7 +1776,7 @@ public:
   pthread_mutex_t mutex;
   pthread_cond_t cond,cond_client;
   volatile uint tables_in_use,stacked_inserts;
-  volatile bool status,dead;
+  volatile bool status;
   COPY_INFO info;
   I_List<delayed_row> rows;
   ulong group_count;
@@ -1784,8 +1784,7 @@ public:
 
   Delayed_insert()
     :locks_in_memory(0),
-     table(0),tables_in_use(0),stacked_inserts(0), status(0), dead(0),
-     group_count(0)
+     table(0),tables_in_use(0),stacked_inserts(0), status(0), group_count(0)
   {
     thd.security_ctx->user=thd.security_ctx->priv_user=(char*) delayed_user;
     thd.security_ctx->host=(char*) my_localhost;
@@ -1934,9 +1933,8 @@ Delayed_insert *find_handler(THD *thd, T
   a given consumer (delayed insert thread), only at different
   stages of producer-consumer relationship.
 
-  'dead' and 'status' variables in Delayed_insert are redundant
-  too, since there is already 'di->thd.killed' and
-  di->stacked_inserts.
+  The 'status' variable in Delayed_insert is redundant
+  too, since there is already di->stacked_inserts.
 */
 
 static
@@ -2089,17 +2087,18 @@ TABLE *Delayed_insert::get_local_table(T
   {
     thd_proc_info(client_thd, "waiting for handler lock");
     pthread_cond_signal(&cond);			// Tell handler to lock table
-    while (!dead && !thd.lock && ! client_thd->killed)
+    while (!thd.killed && !thd.lock && ! client_thd->killed)
     {
       pthread_cond_wait(&cond_client,&mutex);
     }
     thd_proc_info(client_thd, "got handler lock");
     if (client_thd->killed)
       goto error;
-    if (dead)
+    if (thd.killed)
     {
       /* Don't copy over "Server shutdown in progress". */
-      if (thd.stmt_da->sql_errno() == ER_SERVER_SHUTDOWN)
+      if (!thd.is_error() ||
+          thd.stmt_da->sql_errno() == ER_SERVER_SHUTDOWN)
         my_message(ER_QUERY_INTERRUPTED, ER(ER_QUERY_INTERRUPTED), MYF(0));
       else
         my_message(thd.stmt_da->sql_errno(), thd.stmt_da->message(), MYF(0));
@@ -2433,7 +2432,7 @@ static void handle_delayed_insert_impl(T
 	break;					// Time to die
     }
 
-    if (!di->status && !di->stacked_inserts)
+    if (!thd->killed && !di->status && !di->stacked_inserts)
     {
       struct timespec abstime;
       set_timespec(abstime, delayed_insert_timeout);
@@ -2444,7 +2443,7 @@ static void handle_delayed_insert_impl(T
       thd_proc_info(&(di->thd), "Waiting for INSERT");
 
       DBUG_PRINT("info",("Waiting for someone to insert rows"));
-      while (!thd->killed)
+      while (!thd->killed && !di->status)
       {
 	int error;
         mysql_audit_release(thd);
@@ -2461,13 +2460,8 @@ static void handle_delayed_insert_impl(T
 	}
 #endif
 #endif
-	if (thd->killed || di->status)
-	  break;
-	if (error == ETIMEDOUT || error == ETIME)
-	{
-	  thd->killed= THD::KILL_CONNECTION;
-	  break;
-	}
+        if (error == ETIMEDOUT || error == ETIME)
+          thd->killed= THD::KILL_CONNECTION;
       }
       /* We can't lock di->mutex and mysys_var->mutex at the same time */
       pthread_mutex_unlock(&di->mutex);
@@ -2508,14 +2502,12 @@ static void handle_delayed_insert_impl(T
           di->table= 0;
           if (di->open_and_lock_table())
           {
-            di->dead= 1;
             thd->killed= THD::KILL_CONNECTION;
           }
         }
         else
         {
           /* Fatal error */
-          di->dead= 1;
           thd->killed= THD::KILL_CONNECTION;
         }
       }
@@ -2525,9 +2517,8 @@ static void handle_delayed_insert_impl(T
     {
       if (di->handle_inserts())
       {
-	/* Some fatal error */
-	di->dead= 1;
-	thd->killed= THD::KILL_CONNECTION;
+        /* Some fatal error */
+        thd->killed= THD::KILL_CONNECTION;
       }
     }
     di->status=0;
@@ -2575,7 +2566,6 @@ err:
   */
 
   di->table=0;
-  di->dead= 1;                                  // If error
   thd->killed= THD::KILL_CONNECTION;	        // If error
   pthread_mutex_unlock(&di->mutex);
 
@@ -2639,7 +2629,6 @@ end:
   */
 
   di->table=0;
-  di->dead= 1;                                  // If error
   thd->killed= THD::KILL_CONNECTION;	        // If error
   pthread_mutex_unlock(&di->mutex);
 


Attachment: [text/bzr-bundle] bzr/[e-mail]