From: Jon Olav Hauglid Date: August 24 2010 2:00pm Subject: bzr commit into mysql-5.5-bugfixing branch (jon.hauglid:3197) Bug#54332 List-Archive: http://lists.mysql.com/commits/116650 X-Bug: 54332 Message-Id: <201008241401.o7OB9Bmi000338@acsinet15.oracle.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1704510436877991966==" --===============1704510436877991966== 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 14:00:17 +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 reacquire the metadata lock. */ table_list->mdl_request.ticket= NULL; @@ -1839,14 +1836,25 @@ public: mysql_cond_t cond, cond_client; volatile uint tables_in_use,stacked_inserts; volatile bool status; + /* + When the handler thread starts, it clones a metadata lock ticket + for the table to be inserted. This is done to allow the deadlock + detector to detect deadlocks resulting from this lock. + Before this is done, the connection thread cannot safely exit + without causing problems for clone_ticket(). + Once handler_thread_initialized has been set, it is safe for the + connection thread to exit. + Access to handler_thread_initialized is protected by di->mutex. + */ + bool handler_thread_initialized; 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), handler_thread_initialized(FALSE), group_count(0) { DBUG_ENTER("Delayed_insert constructor"); thd.security_ctx->user=thd.security_ctx->priv_user=(char*) delayed_user; @@ -2063,19 +2071,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); @@ -2088,15 +2086,20 @@ bool delayed_get_table(THD *thd, TABLE_L error)); mysql_mutex_unlock(&di->mutex); di->unlock(); - di->thd.mdl_context.release_lock(table_list->mdl_request.ticket); delete di; my_error(ER_CANT_CREATE_THREAD, MYF(ME_FATALERROR), error); goto end_create; } - /* Wait until table is open */ + /* + Wait until table is open unless the handler thread or the connection + thread has been killed. Note that we in all cases must wait until the + handler thread has been properly initialized before exiting. Otherwise + we risk doing clone_ticket() on a ticket that is no longer valid. + */ thd_proc_info(thd, "waiting for handler open"); - while (!di->thd.killed && !di->table && !thd->killed) + while (!di->handler_thread_initialized || + (!di->thd.killed && !di->table && !thd->killed)) { mysql_cond_wait(&di->cond_client, &di->mutex); } @@ -2524,6 +2527,7 @@ pthread_handler_t handle_delayed_insert( /* Can't use my_error since store_globals has not yet been called */ thd->stmt_da->set_error_status(thd, ER_OUT_OF_RESOURCES, ER(ER_OUT_OF_RESOURCES), NULL); + di->handler_thread_initialized= TRUE; } else { @@ -2534,6 +2538,7 @@ pthread_handler_t handle_delayed_insert( /* Can't use my_error since store_globals has perhaps failed */ thd->stmt_da->set_error_status(thd, ER_OUT_OF_RESOURCES, ER(ER_OUT_OF_RESOURCES), NULL); + di->handler_thread_initialized= TRUE; thd->fatal_error(); goto err; } @@ -2546,7 +2551,24 @@ 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)) + { + di->handler_thread_initialized= TRUE; + goto err; + } + + /* + Now that the ticket has been cloned, it is safe for the connection + thread to exit. + */ + di->handler_thread_initialized= TRUE; + di->table_list.mdl_request.ticket= NULL; if (di->open_and_lock_table()) goto err; --===============1704510436877991966== 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: 1530864ae0b01bff82af60f9f4919c107016b797 # timestamp: 2010-08-24 16:00:20 +0200 # source_branch: file:///export/home/x/mysql-5.5-runtime/ # base_revision_id: alfranio.correia@stripped\ # ni0v4isrd4d9ix6y # # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWZjjQwYAA2JfgHQweX///399 nqC////+YAjL73k6XvO7T0bexXk3m7sAap0wTlDIIVTzKPVNtTaifqnsVPyTU8KAGmT1MTTQA9QA GRAmEyBJpGmmjRpk0aaAAaAAAZqDVPTCFNCP1Q9BqaGmQAANAANAGjIBhETTQjKnqm1H6pj0p+qf qm1PUB6IaaaAAAABw00wQyGmmRkwgGmgDCaNMmABA0EiSGgSejRoJpNMTQn6m1NDSDTQ0ZGIA0aR AAP+u08Nud8icSqYP8uv7C6yIx02sK0QdfwlRv7tzfXJ74BX79OHtVFOiAppz2tlHJibNdg/VWgQ 4ZNFUIehHGgNcsuzsKbJ1Ra5Oqn9J7egAQUJIXAz34nxzOutHD43IEJHq/QDyYUxUjaOCiQ6wOcy hwVH6UWSU2srTeJFecXulfwpkt4CEi7jAdNdhZ0Hz0HbM8SvIwpOQkVzTBwalw0oOVoJcvRZlO6Q E7NFGC312CDMqkHTIhSb7u3OpNo0vkgN1/50oNf3wyl4HGTGApEMRDu0Jc5sbEQ2NHSEwsb16Uer rlG9bVp3rRaukJVGNkZAQUsV8FLIKsFsNSxCOwlztpHRzA2T4Na6+Dr20N64KIC0EP+qQiEhvlID g330ZwGKxvwvAnPCEry87uN8Ujz9SS6adnT1tuziGgcfZCFwvMqa7kAx4sBg94Sm6cZl8g73IA1g 7hPk8YkYSNYSYt4UaDWCO8KiJASKk68zazbVK8ZsLAdg80c03Ci0v/cqvBZmUY+bGoJpHgUaQJwg 8YuVbzmIqys0bO/yTWpoVHfFEaDyigRxSJkJi7YV3ZLLyEyYhlheQrVap5DIWnrPh3CEGjbJREle 59dgD9GjHQaZ2zMTAGZtg+6+bwcDSKck8cZLQGOIjmgHJzB2grwVvXUwZCQgLnm2t4aswiYH0OmY NB1bKKXaxYVSnrqxkFFwxZbBKQsNpMIYo10lNdoXLqEYXj/N0c2RAfCRATBra0gCzKavCNNosRoU SaEsOmB0eI7rqrnuZua3FgGxK97V00a9USSk3icI+apcFQUQhacx2IqosDPe9rNqERSKLZmbLO4z OeVp2JYLBtS7eZ26hdsHM4SEwuWRvCCtX110Y4xBGAVjULdIsnPMNoNfHaiIKCqX6wgtrU2GgUPO AtWx3gZX3OkoqVTGyPPFowMgwpBGCIEK32X9nmP3fXTHqN9UwZs5ONClw7u7xJBTzJper46N2FzY Rekkli40MfccYEfzCuDkh7VnH8z1d2t2S+ozSg6co0AGmPJEipCigNueM3jPTxi/OvULSREowBZH hAl+xQ40oKlVK5ptM+rlrCzj5POcKvUu70F4lfjGdIz0JoRJKNskj0mAZqSJh/vuO3BfjnQq1r5N ZbQGtIIUnKXa8UVF5ITTNBWN15qBPclXFfypAgmuXEdTp59oq7YssFJHKgA5oEHpCMlfp9zi4E5D ton9atELeP2r2ePoPw6aLy45qurVqaFEMQQEJF16TSNUEe0v9Mlh0AeosieguxCaFOTbaZZSojES 3AQUNMDrA8DwRpPaOPPxnrtc0PMPKhyIqSMiEJpcBmBg85KMcP8jubol8CiiXAy4AtCQWqwGLDdK XeNptsIfEgXf5aDcUC79MFVMydAuMoqbK1NwfFJUShYAzNjhBFbwuwBlsGrULuxoP8RfhhW1IpW8 zAQmuOJUEjPf5k2gWwO2Ji4FM2upZzVzgrw57ZOEGKFcpFw+UsuCkeIwSMCLqBTy1uQ5O5U1ftRk cpqnTsfgmiqDpmXc6gGMDRDrA5b9tFCC0rtVrIDQjV3r0z80eprzBWjr6ZOyyQKQFSdhQDDUBAXn G3QFsFVegcfJnpEg4gZChRiHc/d/Blr0IwGuchTt3qLyXmnzNBDN9KI30iMeyw2fA59vOyAp0P7G GTXWxg6Wl6KxMCIBL1sRql2uRxdI23KZISLEh84LUoNwMFkSgQbl9fpUARK3F2Elw2rA3hQXEwuM KQw8JO6uKmcN0b7rgYxNMG7D8urbmvViLwfOFuIxgxsNGAa+lbAMakugcF9pYQCpo4cVtwtQuJj7 z1jjKk65tJfMdzEF/gx63zBSHktO9piR9J1hyBRAdKoTQe+QcoWeaxthckRMckqlYEFCAfLet3GG BnzpSQsxdD6mEclF00OahTcnt92dKTwLXZn6FfDSIDJa2UN1/YvGmtCVx4Ze7pXGw0mdLilbCEEg 6wkgY8qIYKRPbFQUuAaQUi2CaqaoQn5A8JFSoQPCAYqe1W9XfWxIsshjJkwBKKM7doHYiL9bNVlc bhA+Q5jzGHD4Vc4K0+dkMa8IDV1wxcHNjFQ112ZWQGoIejT80iAkiTVaxo0spXtLNW6syCNWsYYD pBmsrhvIhRoteDw5hm5c9sCLkx3BMiPogViHlFHIPNChsBttaWghJY61lXcqVtZOMNaEZSvdN6qP HSiDxJaI1cIKa6pQrwy/Gw36a5I011lEi7YQHIazKBaWSHHyMyqItQi477WA0HgWEKtZ8OwFt4rf AXKCV54aAjGktz41jIFsQIWZvTu7ljybGBzKu2SiP4Vv+egC+FPzj4AYFly7whjg81zmscXhAXVM pGApMtw6IWVEk2VQRJxFCkRBrWtBBPdvIS8qZ2xxJUoWGGQxq1nlnbkbdWepZjq5IF7MXlpoEu0a Al54U2epopa50Hu2a5gqUFK8ZDWWdbzRPZRECmixijYFIOtQquwM8svED2r+6EwJ6vBiIDAb4oAY WYp32KbWfeeQqTTFNTewxhlLiplwAmHI5kTyoLkFIN17TIvBQMgO9mKhwGziKBaiRz3b4WUlPpNa d1noNgqbTMJVmbuNHypX/i7kinChITHGhgw= --===============1704510436877991966==--