From: Davi Arnaut Date: December 13 2010 11:11am Subject: bzr commit into mysql-5.5-bugteam branch (davi:3185) Bug#58136 List-Archive: http://lists.mysql.com/commits/126613 X-Bug: 58136 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6818192816792081691==" --===============6818192816792081691== MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline # 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; --===============6818192816792081691== MIME-Version: 1.0 Content-Type: text/bzr-bundle; charset="us-ascii"; name="bzr/davi.arnaut@stripped" Content-Transfer-Encoding: 7bit Content-Disposition: inline # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: davi.arnaut@stripped # target_branch: file:///home/davi/bzr/bugs/58136-5.5/ # testament_sha1: 546d4fd955eec67230038369856c84733590cd46 # timestamp: 2010-12-13 09:11:51 -0200 # base_revision_id: davi.arnaut@stripped\ # xqxqgaqqxash7jhj # # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWYDhELkABf7fgFUQeXf//3/v v6C////6YAw/U89ZuvvKeAoUAWszJls0esrrVSiKm2EBoij0mjJoGo09TaHqNAmCNDIxMAAEYhw0 0wQyGmmRkwgGmgDCaNMmABA0Gmo0p5JohmU02k09R+qABphBkNAAAACREINUbSaif6o0eqM9U/VP UZDxIDQ0yZAaNpABFJNE00aNNAIaZTZBNNEabSNGg0BoBoaCSIE0BMmgENCaNKe1T0nqAaGmgD1N MQ8U0AJTR9jAOvp3H26v2sju+qNsju6HqsnwvIARBvizp2rGfipTa0EGgYHJuByQlZ95MVJD1QlL cREtkG0VV2vDxjs9v49Lj5kuEBF7JmBdeLoKl5i3LAd2d+VcrIxsItIhSe2V0JmGvVJBFIyYCxhD TzNx+gjQ9gv1FuE220Ng2m20uvwfuC49r3MKHCNO7BvZPc54gkyYYplHnLdbLPHGcCr3PJ87cQsW 8Yo6XY4vMpcZYVrCN8WYNYX0S56ubn3KZ9NYB6oXJF5c+h1eTPPTGtt9It4ZtnWCc4nhsk7PZb27 SuTzXv6RyVrGJDglkqbcHmYFJwvhPZjb5wwnXP8FfnutrfI8hRroXOU/HSe2wU9G2QScsIWwNLM9 z3zqChPGkLNkIQxvIaZ0uo07IQYikKZArVUDQMswLw6o2m7ExbvRryS1beyCREFSqApgnc2nHQQU YBFNri+KIpPPk7MDDDwUe6GAHzYK68OkW2ombf13eXmBQWwhuYCRZsIKlYoNuZtlWFJ6tCUm8gCH sQdwB3D5+qw4PWbn0F26/DUcnJJ8uIZoJVxrKmWWEzSKDezfHHq2gibY/FI1KkcStyYIrrWqaung e6u/vaVdMxX5m0PUjWMAXjYnBoh1wgaYJiSanmok6hIQZizgU0hDhY3HiJKCMVa18qnydW0xMyxo ZGgXNgigQwuu9gVGR0Gdb1IAMHjzJKGiKS70pTCwA9jCQY2JxGBoac6gH76DOAZiCxGECCdYkvtk q24VNBkglUx7oy37nxs7d1sM9wyAhsIog3AYhlBYyVtoFtAavDPeWrNUgPlmOvhjQ3mZfRyebrce lhTAZHAQyrysNotRtTNaiuKcoAugB/0+hMwVA34ubuYZjsCWackORGXM8nAG4akblcc5biRgTfiP gUyvx3l28e26ZazcbICLyRsDIOwmqmJ6gDBdVmLHDa0MLxyHNbiIiAgvMFAyhjVVUTUozBpjUvm7 ZyklLNagDzo8nt3j3KZlAkQpiRoRFA7VntIk7Wb4K4WR1MhfJcjEmW1vzvNvCdEs044p1KAV+sAf dY4XSLU8Oagq8+ErHMalExoRsDEYkWZAzLwI5pevQ2EChVw+QvaeBd2GL52tkZwbjgrLImk3KjyM 1uJGk6uh8GBGgoKPQuqta3qW7Y+WiZVwtF0ejYZCO4ly3Dl5Iga/NRO0Y7yZhQ4QIVyzWeRlwsyL NRiEFVQXlrfPGxzDj+V7S1ymJkOWPxVpMcuOMQKxImWhE2bL9Tag7ipm0hlsBX5mF5SAS6ROpqLU s1TrQnu2EgkMGBjQvMzW2Drxw3yiBcVLSp32+Sz+iiuiZXqoBM/XnDFjgnZcYG2BGIywMTibysVX gSLamoxK0kdhKGT7TEmUIrddbK5pwok6ZX7i8tJEStFhO4vMqRjM2DrARgSaRbA2jCXEbCBAsjMf bpVrDIkObNg9mGE8oH1qeFGfskQm9AaIxDVU1aYZRCCVaQAkncq8ErqTejIMcn5A+pnYxG9nP1pS dp+rbw1FC6xWgkgEw4+I8u+PVuMDYCZG598sV0QkBEMevd7E/oPi4ICpWT2CoTI/MXecHG1kv4Rd Gd0di9CC69pB/i+XzWy9rw8FbMaX4Gs1Kfz/8qZHyTbhikRygaorfudPukoaomiSd6VQ6KbkKKtM oI+kLrfiQB1eZMaTMJB8L6S+1TK1BD8aG0uwRGwZJT0J0NT0bwO5hkDwCSwSe2PIMC+2lOPtpR5B TUZwozqgHYS+i58fR5CSx5T5iD5z1kmR5SpievQ+w/bJpI8244OwfdEIfcCyD06n4GlCq74oCrwO 6eIj3v3XBZbJX2fiCSbcNWLpeAFpVBbZJjcmYMTRZdgdN6J44QSZ9/D3bjUSfsOo92R4jvIOcmFt 1RHd4jHS5/ZxM9x/ipiqFQPiyQXiWRgYHr2Gm03FhiX3Nn4LeeAlkMeBpHC0uNv8lcbm4BqquoN6 wKFS6UEEP+bGCCJZ4TMI7ZPUBgVk9JaZVdghg4NVz7q2rbbaJY5IHxs5HTmxtigsw0E6ssEewzy4 GBwLpnQ7DGJe3QzA7o4tiBAjLtVThSoBtCmkzqcQ41X9D3JMJ/nujCpuK8zx46Gi0uw9xLuWZjI+ ETatO9tKSX6uz37eEJHTLX2yd4x6NCb2sD+9nkcPyED3jRkb7S2PnWakhL6jt/1idTuLLj76M1Mn 3E9fcXF49xqLMXfX4OJ9ELRBagxPQ0De3NTzNnqO8BY7cKRiRZ4PX0ldLJa8nvJ38GyyfBPipBCM 03BLkkkeBT3PQ4U4qqXfE0OLivUz4IgZACzUTAcSmvd3sdYZwketeiWA2UTzxDrBPzTLhZsQRuS0 14oNMSeJxB8nR0EyXSzOT4WDjV4ET725pDNB3Ul0L3vbkJr37RuK3zD2+cYbRtucQ7Sc4KWxfqcl MPJtRB0PvXVdFmuFL0L1cD0L17Cgqit8Y8FjU9b2FLOBdXuirA3mwoQfqhghiA20ytLipPjXWAd4 Anwjq1r0gGJ4ARrUWyAR+MnKzeJ1moNRu2E5Zg8DrhW5lj48z55Lh462DMKaT6lUKg6CO+7PUaXg b9Kv591vJfcMMzMItxAGUrdhBEGfQfPEQGBCborC+brsGNBzMY3OdrYZq0yVgebNgJsV0QR81xXJ dTM8gUA9DVBGA+usqVMIGdkJmmuulUkrFb6eGsAyN5gOxk98gDqa6+L4HtYEY2NTgU8efuJcMR0C 6DQdbIpmFDeEIGT8h4F/cK0tGFK6dUq3CyLdDuX0bDx4PrlEYpfJjz/lfIiIaCclDWAQtg3InAv7 SQ9WaqBtrQZ7OSSOU6e08ZxSrZtagCwSmSvF7z4EhYlqZmSGGA9Q66LQ+BIt5rurYaslUNXi0RmZ maXaKMYsh+oXh4+BE8FqF6wAPaHXuMVeEFxgmpgCRoxSVKfFgL44qmRCjPDbQ3qmcwRzUtFNj2Uv ld0sWsjOIajNcQcoGAOxhqX0XWch2HytXfVod+akbccHM52x1j1JoRRy8VyzPDNUVqgtBibuzK9w AgGB4+68lJuZySjZpksQBh5sS87ac0tuUr1mRyj98wS1dg4nXcd2HvjRMea7SwRamMPWQde/ywY2 sDsSo8PVPvAjFw38GeKClloskwlPPg/GsoKgRSzoLoTKXMMCguVG1rgVC/SLpfJERENt8guKoplU iBwUngQ1E5khMoF6RDARWJyNrM258/k30xPS0ucSEmZk4zMrj3eRK0ETFL1goo0N0o3SDvocalQW GQBgrFE3oPxHOgMfRhLBr2P3N6CrLviciMMn3gFDXGqqHVb0ElGqlAs4l0AHxlTvLAPu5F4dn2Zq pmWrpHEl2ePGGupRCJHZovWfAUV0l+s9EEnZc2mNEyH0Q3RKJb5PzVocXuZ6ihscTygpN5BXtpu6 2hhmbms532gGcoJvOdrgRwFjueD03VxFlFFETcZVMy+k3K4XI5DLauli98xfU+Vyldz6jaf+LuSK cKEhAcIhcg== --===============6818192816792081691==--