List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:December 15 2010 12:28pm
Subject:bzr commit into mysql-5.5-bugteam branch (davi:3196) Bug#58136
View as plain text  
# At a local mysql-5.5-bugteam repository of davi

 3196 Davi Arnaut	2010-12-15
      Bug#58136: Crash in vio_close at concurrent disconnect and KILL
      
      The problem is a race between a session closing its vio
      (i.e. after a COM_QUIT) at the same time it is being killed by
      another thread. This could trigger a assertion in vio_close()
      as the two threads could end up closing the same vio, at the
      same time. This could happen due to the implementation of
      SIGNAL_WITH_VIO_CLOSE, which closes the vio of the thread
      being killed.
      
      The solution is to serialize the close of the Vio under
      LOCK_thd_data, which protects THD data.
     @ sql/mysqld.cc
        Drop lock parameter from close_connection, its not necessary
        any more. The newly introduced THD::disconnect method will take
        care of locking.
     @ sql/mysqld.h
        Change prototype, add a default parameter for the error code.
     @ sql/sql_class.cc
        In case SIGNAL_WITH_VIO_CLOSE is defined, the active vio is
        closed and cleared. Subsequent calls will only close the vio
        owned by the session.

    modified:
      sql/mysqld.cc
      sql/mysqld.h
      sql/sql_class.cc
      sql/sql_class.h
      sql/sql_connect.cc
      sql/sql_parse.cc
=== modified file 'sql/mysqld.cc'
--- a/sql/mysqld.cc	2010-12-14 14:34:23 +0000
+++ b/sql/mysqld.cc	2010-12-15 12:27:59 +0000
@@ -1134,7 +1134,7 @@ static void close_connections(void)
                           tmp->thread_id,
                           (tmp->main_security_ctx.user ?
                            tmp->main_security_ctx.user : ""));
-      close_connection(tmp,0,0);
+      close_connection(tmp);
     }
 #endif
     DBUG_PRINT("quit",("Unlocking LOCK_thread_count"));
@@ -1960,39 +1960,28 @@ static void network_init(void)
 /**
   Close a connection.
 
-  @param thd		Thread handle
-  @param errcode	Error code to print to console
-  @param lock	        1 if we have have to lock LOCK_thread_count
+  @param thd        Thread handle.
+  @param sql_errno  The error code to send before disconnect.
 
   @note
     For the connection that is doing shutdown, this is called twice
 */
-void close_connection(THD *thd, uint errcode, bool lock)
+void close_connection(THD *thd, uint sql_errno)
 {
-  st_vio *vio;
   DBUG_ENTER("close_connection");
-  DBUG_PRINT("enter",("fd: %s  error: '%s'",
-		      thd->net.vio ? vio_description(thd->net.vio) :
-		      "(not connected)",
-		      errcode ? ER_DEFAULT(errcode) : ""));
-  if (lock)
-    mysql_mutex_lock(&LOCK_thread_count);
-  thd->killed= THD::KILL_CONNECTION;
-  if ((vio= thd->net.vio) != 0)
-  {
-    if (errcode)
-      net_send_error(thd, errcode,
-                     ER_DEFAULT(errcode), NULL); /* purecov: inspected */
-    vio_close(vio);			/* vio is freed in delete thd */
-  }
-  if (lock)
-    mysql_mutex_unlock(&LOCK_thread_count);
-  MYSQL_CONNECTION_DONE((int) errcode, thd->thread_id);
+
+  if (sql_errno)
+    net_send_error(thd, sql_errno, ER_DEFAULT(sql_errno), NULL);
+
+  thd->disconnect();
+
+  MYSQL_CONNECTION_DONE((int) sql_errno, thd->thread_id);
+
   if (MYSQL_CONNECTION_DONE_ENABLED())
   {
     sleep(0); /* Workaround to avoid tailcall optimisation */
   }
-  MYSQL_AUDIT_NOTIFY_CONNECTION_DISCONNECT(thd, errcode);
+  MYSQL_AUDIT_NOTIFY_CONNECTION_DISCONNECT(thd, sql_errno);
   DBUG_VOID_RETURN;
 }
 #endif /* EMBEDDED_LIBRARY */
@@ -4951,8 +4940,8 @@ void create_thread_to_handle_connection(
       my_snprintf(error_message_buff, sizeof(error_message_buff),
                   ER_THD(thd, ER_CANT_CREATE_THREAD), error);
       net_send_error(thd, ER_CANT_CREATE_THREAD, error_message_buff, NULL);
+      close_connection(thd);
       mysql_mutex_lock(&LOCK_thread_count);
-      close_connection(thd,0,0);
       delete thd;
       mysql_mutex_unlock(&LOCK_thread_count);
       return;
@@ -4993,7 +4982,7 @@ static void create_new_thread(THD *thd)
     mysql_mutex_unlock(&LOCK_connection_count);
 
     DBUG_PRINT("error",("Too many connections"));
-    close_connection(thd, ER_CON_COUNT_ERROR, 1);
+    close_connection(thd, ER_CON_COUNT_ERROR);
     delete thd;
     DBUG_VOID_RETURN;
   }
@@ -5374,7 +5363,7 @@ pthread_handler_t handle_connections_nam
     if (!(thd->net.vio= vio_new_win32pipe(hConnectedPipe)) ||
 	my_net_init(&thd->net, thd->net.vio))
     {
-      close_connection(thd, ER_OUT_OF_RESOURCES, 1);
+      close_connection(thd, ER_OUT_OF_RESOURCES);
       delete thd;
       continue;
     }
@@ -5569,7 +5558,7 @@ pthread_handler_t handle_connections_sha
                                                    event_conn_closed)) ||
                         my_net_init(&thd->net, thd->net.vio))
     {
-      close_connection(thd, ER_OUT_OF_RESOURCES, 1);
+      close_connection(thd, ER_OUT_OF_RESOURCES);
       errmsg= 0;
       goto errorconn;
     }

=== modified file 'sql/mysqld.h'
--- a/sql/mysqld.h	2010-12-07 17:08:54 +0000
+++ b/sql/mysqld.h	2010-12-15 12:27:59 +0000
@@ -64,7 +64,7 @@ typedef Bitmap<((MAX_INDEXES+7)/8*8)> ke
                                            some places */
 /* Function prototypes */
 void kill_mysql(void);
-void close_connection(THD *thd, uint errcode, bool lock);
+void close_connection(THD *thd, uint sql_errno= 0);
 void handle_connection_in_main_thread(THD *thd);
 void create_thread_to_handle_connection(THD *thd);
 void unlink_thd(THD *thd);

=== modified file 'sql/sql_class.cc'
--- a/sql/sql_class.cc	2010-11-18 15:01:58 +0000
+++ b/sql/sql_class.cc	2010-12-15 12:27:59 +0000
@@ -1283,6 +1283,35 @@ void THD::awake(THD::killed_state state_
   DBUG_VOID_RETURN;
 }
 
+
+/**
+  Close the Vio associated this session.
+
+  @remark LOCK_thd_data is taken due to the fact that
+          the Vio might be disassociated concurrently.
+*/
+
+void THD::disconnect()
+{
+  Vio *vio= NULL;
+
+  mysql_mutex_lock(&LOCK_thd_data);
+
+  killed= THD::KILL_CONNECTION;
+
+#ifdef SIGNAL_WITH_VIO_CLOSE
+  vio= active_vio;
+  close_active_vio();
+#endif
+
+  /* Disconnect even if a active vio is not associated. */
+  if (net.vio != vio)
+    vio_close(net.vio);
+
+  mysql_mutex_unlock(&LOCK_thd_data);
+}
+
+
 /*
   Remember the location of thread info, the structure needed for
   sql_alloc() and the structure for the net buffer

=== modified file 'sql/sql_class.h'
--- a/sql/sql_class.h	2010-12-14 10:46:00 +0000
+++ b/sql/sql_class.h	2010-12-15 12:27:59 +0000
@@ -2209,6 +2209,9 @@ public:
 #endif
   void awake(THD::killed_state state_to_set);
 
+  /** Disconnect the associated communication endpoint. */
+  void disconnect();
+
 #ifndef MYSQL_CLIENT
   enum enum_binlog_query_type {
     /* The query can be logged in row format or in statement format. */

=== modified file 'sql/sql_connect.cc'
--- a/sql/sql_connect.cc	2010-12-08 16:47:21 +0000
+++ b/sql/sql_connect.cc	2010-12-15 12:27:59 +0000
@@ -518,7 +518,7 @@ bool setup_connection_thread_globals(THD
 {
   if (thd->store_globals())
   {
-    close_connection(thd, ER_OUT_OF_RESOURCES, 1);
+    close_connection(thd, ER_OUT_OF_RESOURCES);
     statistic_increment(aborted_connects,&LOCK_status);
     MYSQL_CALLBACK(thread_scheduler, end_thread, (thd, 0));
     return 1;                                   // Error
@@ -693,7 +693,7 @@ void do_handle_one_connection(THD *thd_a
 
   if (MYSQL_CALLBACK_ELSE(thread_scheduler, init_new_connection_thread, (), 0))
   {
-    close_connection(thd, ER_OUT_OF_RESOURCES, 1);
+    close_connection(thd, ER_OUT_OF_RESOURCES);
     statistic_increment(aborted_connects,&LOCK_status);
     MYSQL_CALLBACK(thread_scheduler, end_thread, (thd, 0));
     return;
@@ -751,7 +751,7 @@ void do_handle_one_connection(THD *thd_a
     end_connection(thd);
    
 end_thread:
-    close_connection(thd, 0, 1);
+    close_connection(thd);
     if (MYSQL_CALLBACK_ELSE(thread_scheduler, end_thread, (thd, 1), 0))
       return;                                 // Probably no-threads
 

=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc	2010-12-08 16:47:21 +0000
+++ b/sql/sql_parse.cc	2010-12-15 12:27:59 +0000
@@ -600,7 +600,7 @@ void do_handle_bootstrap(THD *thd)
   if (my_thread_init() || thd->store_globals())
   {
 #ifndef EMBEDDED_LIBRARY
-    close_connection(thd, ER_OUT_OF_RESOURCES, 1);
+    close_connection(thd, ER_OUT_OF_RESOURCES);
 #endif
     thd->fatal_error();
     goto end;


Attachment: [text/bzr-bundle] bzr/davi.arnaut@oracle.com-20101215122759-n8eshqc0jwam2p44.bundle
Thread
bzr commit into mysql-5.5-bugteam branch (davi:3196) Bug#58136Davi Arnaut15 Dec
Re: bzr commit into mysql-5.5-bugteam branch (davi:3196) Bug#58136Jon Olav Hauglid15 Dec