From: Jon Olav Hauglid Date: August 24 2010 12:49pm Subject: bzr commit into mysql-5.5-bugfixing branch (jon.hauglid:3197) Bug#54332 List-Archive: http://lists.mysql.com/commits/116631 X-Bug: 54332 Message-Id: <201008241251.o7O6SoBh002392@acsinet15.oracle.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0774059909368514291==" --===============0774059909368514291== MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline #At file:///export/home/x/mysql-5.5-bugfixing-test/ based on revid:alfranio.correia@stripped 3197 Jon Olav Hauglid 2010-08-24 Follow-up to Bug #54332 Deadlock with two connections doing LOCK TABLE+INSERT DELAYED The problem was that the server could crash if the insert delayed handler thread was killed due to a conflicting shared metadata lock. This could happen because the metadata lock ticket was added to the handler thread before it was properly initialized. This patch moves the cloning of the acquired metadata lock ticket until after the handler thread has been properly initialized. modified: sql/sql_insert.cc === modified file 'sql/sql_insert.cc' --- a/sql/sql_insert.cc 2010-08-23 15:42:53 +0000 +++ b/sql/sql_insert.cc 2010-08-24 12:49:25 +0000 @@ -563,15 +563,6 @@ bool open_and_lock_for_insert_delayed(TH */ DBUG_RETURN(TRUE); - /* - If a lock was acquired above, we should release it after delayed_get_table() - has cloned the ticket for the handler thread. Note that acquire_lock() can - succeed because of a lock already held by the connection. In this case we - should not release it here. - */ - MDL_ticket *table_ticket = mdl_savepoint == thd->mdl_context.mdl_savepoint() ? - NULL: thd->mdl_context.mdl_savepoint(); - bool error= FALSE; if (delayed_get_table(thd, table_list)) error= TRUE; @@ -597,12 +588,18 @@ bool open_and_lock_for_insert_delayed(TH } } - if (table_ticket) - thd->mdl_context.release_lock(table_ticket); /* - Clone_ticket() in delayed_get_table() causes TABLE_LIST::MDL_REQUEST::ticket - to be overwritten with the cloned ticket. Reset the ticket here in case - we end up having to use normal insert. + If a lock was acquired above, we should release it after + handle_delayed_insert() has cloned the ticket. Note that acquire_lock() can + succeed because the connection already has the lock. In this case the ticket + will be before the mdl_savepoint and we should not release it here. + */ + if (!thd->mdl_context.has_lock(mdl_savepoint, table_list->mdl_request.ticket)) + thd->mdl_context.release_lock(table_list->mdl_request.ticket); + + /* + Reset the ticket in case we end up having to use normal insert and + therefore will reopen the table and reaquire the metadata lock. */ table_list->mdl_request.ticket= NULL; @@ -1838,15 +1835,15 @@ public: mysql_mutex_t mutex; mysql_cond_t cond, cond_client; volatile uint tables_in_use,stacked_inserts; - volatile bool status; + volatile bool status, ticket_was_cloned; COPY_INFO info; I_List rows; ulong group_count; TABLE_LIST table_list; // Argument Delayed_insert() - :locks_in_memory(0), - table(0),tables_in_use(0),stacked_inserts(0), status(0), group_count(0) + :locks_in_memory(0), table(0),tables_in_use(0),stacked_inserts(0), + status(0), ticket_was_cloned(FALSE), group_count(0) { DBUG_ENTER("Delayed_insert constructor"); thd.security_ctx->user=thd.security_ctx->priv_user=(char*) delayed_user; @@ -2063,19 +2060,9 @@ bool delayed_get_table(THD *thd, TABLE_L /* Replace volatile strings with local copies */ di->table_list.alias= di->table_list.table_name= di->thd.query(); di->table_list.db= di->thd.db; - - /* - Clone the ticket representing the lock on the target table for - the insert and add it to the list of granted metadata locks held by - the handler thread. This is safe since the handler thread is - not holding nor waiting on any metadata locks. - */ - if (di->thd.mdl_context.clone_ticket(&table_list->mdl_request)) - { - delete di; - my_error(ER_OUT_OF_RESOURCES, MYF(ME_FATALERROR)); - goto end_create; - } + /* We need the ticket so that it can be cloned in handle_delayed_insert */ + init_mdl_requests(&di->table_list); + di->table_list.mdl_request.ticket= table_list->mdl_request.ticket; di->lock(); mysql_mutex_lock(&di->mutex); @@ -2096,7 +2083,7 @@ bool delayed_get_table(THD *thd, TABLE_L /* Wait until table is open */ thd_proc_info(thd, "waiting for handler open"); - while (!di->thd.killed && !di->table && !thd->killed) + while (!di->ticket_was_cloned || (!di->thd.killed && !di->table && !thd->killed)) { mysql_cond_wait(&di->cond_client, &di->mutex); } @@ -2546,7 +2533,17 @@ pthread_handler_t handle_delayed_insert( thd->lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_INSERT_DELAYED); thd->set_current_stmt_binlog_format_row_if_mixed(); - init_mdl_requests(&di->table_list); + /* + Clone the ticket representing the lock on the target table for + the insert and add it to the list of granted metadata locks held by + the handler thread. This is safe since the handler thread is + not holding nor waiting on any metadata locks. + */ + if (thd->mdl_context.clone_ticket(&di->table_list.mdl_request)) + goto err; + + di->ticket_was_cloned= TRUE; + di->table_list.mdl_request.ticket= NULL; if (di->open_and_lock_table()) goto err; --===============0774059909368514291== MIME-Version: 1.0 Content-Type: text/bzr-bundle; charset="us-ascii"; name="bzr/jon.hauglid@stripped" Content-Transfer-Encoding: 7bit Content-Disposition: inline # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: jon.hauglid@stripped # target_branch: file:///export/home/x/mysql-5.5-bugfixing-test/ # testament_sha1: 6507e95500d54b83b57503b9005b04ef88eb6888 # timestamp: 2010-08-24 14:49:29 +0200 # source_branch: file:///export/home/x/mysql-5.5-runtime/ # base_revision_id: alfranio.correia@stripped\ # ni0v4isrd4d9ix6y # # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWXj6PloAAqPfgHAweX///399 nqC////0YAcvvAtXao7ve9de27s6dNda5wsQkkIjCp+aplPTGgCTTTNT1MmQ9RptINqD1NHqAyQj ExMgJqCPKNHqPU2o0AaDRoAAyEUxBM1ABo000D1AZMQ0aaDQD1AADTRNBGiaaCmaJg0EyDJoANAa DTQaHGTJoYjE0YBGAmEAYCaaNMjQDCSIJomQNNFT8Tap6J6mAmpskAYRoA0MkgCyHLcs0QizNB+7 b3mTFK2O+TXPxj+1Xfvy6J55be/hRVcjHPSQYWRLGS5MKlNHKTS6oVrpj754cwCCKEuRoyfDuPXm NKJx9dSEJTv/YCMJsM+LQzl8/jaBjBmlF23jKSzkuXg+UP5q6Th72Ya56bM25sM3ZaogVYwLq4O9 GDup3TTghOSq7kdnDXktcCQrHtstG9yyYxGFA0lcsWIwAJBYEGMt+GsvNhhjGyRyTE6mIeNCkHVc BoaQOaeYs1Vg1jwSlQqmoaaBrihNRisZUyNbjZGx17aNartLzAncrw693JsaUUoKcYIsBTRlvIXZ OAwjEGupOiwnYS9jA52h9NZKtSa21y82CMS0KlgBsS5ocqv3rWUs4c56pLLt176WdgrIBIkZ06IE rkHIsBh63T0139eOWoFrxE7+AwiaRIdd/xcvrIsTJiGlrGdrTlMoZpYZhZVifUDmJmeufbbBdrFE dJjpnrT4TDBIgWWd/UMZ8gYaPTljdlKpW9oG/2CvMQfNQjkgqFbVaNpXLXkEOi6VNjJ0GDi5T2in bBW027a2L3B4cMwZ4dYjKR49VmMAuIQCiwsKcODJttZrcQG0Yqg224y1b5V0LLxjdmlOgqhx5E54 L/vFZNFHMtUBbmYLvk5fVm2nuqHDS5rX5HQODjjUl4ynxBguXCFFYhzA2sPRsUbpdpNRFV3WW3i0 WXnX4LbjU2iW2nKycpBAHAh7FoeyzAdrZlFsojU+IWPDIUS8URQNApwgNFBZqGUXYlLqSF1KoS1K mKctjBnZDIsKASmd/yV8X46Ie8sklYsq1wT8XXq86SksTJbN0ap2YZvDijv7HvPG3+7fDglnwSDm GTsWS3KPO8KgoJSkVH17TBcQCGQiu4AdQ2UwtGGkHAmmgCg0ZSSj7XIkpOithmXiTDvKdvaete2g F9qt5IWZELBVgsusYErA6dywdZia7oc4ewC4lQO9vY+ZFVxAZMwNIoNS6KbAVHwrP8mA5WRiQctH r6b7ex9xL23UjY3UKDsD/nDEIiqaTJQbB1oMJ7l7o85jjG3bHzX6kh3ZAOnSLLT5AzpGDqCglCwR Y3jdFFDFS+oKVUF1RoFUWBVNoDOog2nbjnVG2ddpaTsuVUlpIZUi8wxHiuD6jx3GrY8dzF/mBaEq 8mbe8OBUbW2MzmMBxbjc4vlByRxc4xwqdmnURBbEs+a1AHubdvTBa+X6DBQ16bMBgqRiAhNtMFEY e1Q6d4OMrQouLwFrtvkRyBRMfXk47X5om26ZF0SmS/uaXxczqU1nw6JtxkYcE4MNETJZoNRFmEMh ShW4DsUp0WeV3ETZHvRuscFIVQTE9D7OQ+8L8Twbk4zWadJMcf804b2bWz60xXYqR1cuFhObBIjB IIjqnPD9jE4dOMB2aAT72WDEGXqqTAO4I+1gL64kPS0lCogigDcANSc2AxeQdAbDy8k4Dwszs8L2 pmZjgKxANMBOoCfJBrjfnpAQQoQ4dGvVGBhaDbws0g2UMVrAgiky9zh0+pbLq12Gf4H6DPfI50L5 jZGQG7o0+neEl116W5vWcgiK8JQDipJkE/CAWA/qnZhihIdaGelKpk6CGoWDoQRrSgqSw52V0JhZ UnJsbPyzAZcjZRdZnuEYiaAYLWyYX3cNrX4uEOzot5r40mLqBWWSLFgQ3QQEYSLgOXBkomYYUmIo O0PQQmorwFAmdjb7ozEb2tJoKDwgyZdgpOLoPQ2hlKr1GISaozdJXd1bs4L/UzDdQDK0Dum7EGXn y62Wrm+aRqCCgyqWmOpiR79W9oxCQk+fEFaFWOxEKME48IKHdiGcb2scX32CBD+dpBsoVSRaAqQ0 USaZHBthqGpHKxIxuXFKolg21EKOJEwzcsA84kVAz66aoqmMpIQgJIF9EJNPKrEkuaDZDsAYLDVd nUJcIxE0IJXC1AJtDicScCoC2prhz85ntqomlpJRCg/Z7prUA+dfSGG1gKKMqqdLvoZkYwJLxWJK lCrJ0iCEoGSLxcQAgE6IlE8aKM31goxA4Ng+yrtjO9heNlij0TTCjAeYISYML8c+esiLf5x2VbjY ZabCnoixUWMtQRBqDqdCWM8dg3mS9/VRBALi4IG5+m3dC3eRqMOT+KqvUDVCAoYhYV8ZuKwPF1w4 OZeIsxuWgbXlF3D6dtC+B3bBkUg/xBjxEBojzDqu/4u5IpwoSDx9Hy0A --===============0774059909368514291==--