# At a local mysql-5.5-bugteam repository of davi
3185 Davi Arnaut 2010-12-13
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-07 17:08:54 +0000
+++ b/sql/mysqld.cc 2010-12-13 11:11:41 +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,34 +1960,23 @@ 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 */
@@ -4950,8 +4939,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;
@@ -4992,7 +4981,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;
}
@@ -5373,7 +5362,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;
}
@@ -5568,7 +5557,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-13 11:11:41 +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-13 11:11:41 +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-11-18 15:01:58 +0000
+++ b/sql/sql_class.h 2010-12-13 11:11:41 +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-11-25 03:50:16 +0000
+++ b/sql/sql_connect.cc 2010-12-13 11:11:41 +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;
@@ -748,7 +748,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-11-18 15:01:58 +0000
+++ b/sql/sql_parse.cc 2010-12-13 11:11:41 +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-20101213111141-37ohln1xg2hlhtf4.bundle
| Thread |
|---|
| • bzr commit into mysql-5.5-bugteam branch (davi:3185) Bug#58136 | Davi Arnaut | 13 Dec |
