From: Jon Olav Hauglid Date: July 21 2010 3:00pm Subject: bzr commit into mysql-trunk-runtime branch (jon.hauglid:3081) Bug#55223 List-Archive: http://lists.mysql.com/commits/114049 X-Bug: 55223 Message-Id: <201007211500.o6LAhD1G017242@acsinet15.oracle.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0983399880829523923==" --===============0983399880829523923== MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline #At file:///export/home/x/mysql-trunk-runtime-bug55223/ based on revid:jon.hauglid@stripped 3081 Jon Olav Hauglid 2010-07-21 Bug #55223 assert in Protocol::end_statement during CREATE DATABASE The problem was that a statement could cause an assert if it was aborted by KILL QUERY while it waited on a metadata lock. This assert checks that a statement either sends OK or an error to the client. If the bug was triggered on release builds, it caused OK to be sent to the client instead of ER_QUERY_INTERRUPTED. The root cause of the problem is that there are two separate ways to tell if a statement is killed: thd->killed and mysys_var->abort. KILL QUERY causes both to be set, thd->killed before mysys_var->abort. The latter was used to check if a wait for a metadata lock should be aborted. If waiting is aborted, no OK message is sent and thd->killed is checked to see if ER_QUERY_INTERRUPTED should be sent to the client. The assert could be triggered if the compiler reordered the KILL QUERY code so that mysys_var->abort was set before thd->killed, and the aborted statement reached the thd->killed check before thd->killed was set by KILL QUERY. This patch fixes the problem by changing the metadata lock waiting code to check thd->killed. No test case added as reproducing the assert is dependent on very exact timing of two (or more) threads and compiler reording of instructions. The patch has been checked using RQG and the grammar posted on the bug report. modified: sql/mdl.cc sql/mdl.h === modified file 'sql/mdl.cc' --- a/sql/mdl.cc 2010-07-13 18:01:54 +0000 +++ b/sql/mdl.cc 2010-07-21 15:00:01 +0000 @@ -881,67 +881,6 @@ uint MDL_ticket::get_deadlock_weight() c } -/** - Helper functions and macros to be used for killable waiting in metadata - locking subsystem. - - @sa THD::enter_cond()/exit_cond()/killed. - - @note We can't use THD::enter_cond()/exit_cond()/killed directly here - since this will make metadata subsystem dependent on THD class - and thus prevent us from writing unit tests for it. And usage of - wrapper functions to access THD::killed/enter_cond()/exit_cond() - will probably introduce too much overhead. -*/ - -#define MDL_ENTER_COND(A, B, C, D) \ - mdl_enter_cond(A, B, C, D, __func__, __FILE__, __LINE__) - -static inline const char *mdl_enter_cond(THD *thd, - st_my_thread_var *mysys_var, - mysql_cond_t *cond, - mysql_mutex_t *mutex, - const char *calling_func, - const char *calling_file, - const unsigned int calling_line) -{ - mysql_mutex_assert_owner(mutex); - - mysys_var->current_mutex= mutex; - mysys_var->current_cond= cond; - - DEBUG_SYNC(thd, "mdl_enter_cond"); - - return set_thd_proc_info(thd, "Waiting for table", - calling_func, calling_file, calling_line); -} - -#define MDL_EXIT_COND(A, B, C, D) \ - mdl_exit_cond(A, B, C, D, __func__, __FILE__, __LINE__) - -static inline void mdl_exit_cond(THD *thd, - st_my_thread_var *mysys_var, - mysql_mutex_t *mutex, - const char* old_msg, - const char *calling_func, - const char *calling_file, - const unsigned int calling_line) -{ - DBUG_ASSERT(mutex == mysys_var->current_mutex); - - mysql_mutex_unlock(mutex); - mysql_mutex_lock(&mysys_var->mutex); - mysys_var->current_mutex= NULL; - mysys_var->current_cond= NULL; - mysql_mutex_unlock(&mysys_var->mutex); - - DEBUG_SYNC(thd, "mdl_exit_cond"); - - (void) set_thd_proc_info(thd, old_msg, calling_func, - calling_file, calling_line); -} - - /** Construct an empty wait slot. */ MDL_wait::MDL_wait() @@ -1021,15 +960,14 @@ MDL_wait::timed_wait(THD *thd, struct ti { const char *old_msg; enum_wait_status result; - st_my_thread_var *mysys_var= my_thread_var; int wait_result= 0; mysql_mutex_lock(&m_LOCK_wait_status); - old_msg= MDL_ENTER_COND(thd, mysys_var, &m_COND_wait_status, - &m_LOCK_wait_status); + old_msg= thd_enter_cond(thd, &m_COND_wait_status, &m_LOCK_wait_status, + "Waiting for table"); - while (!m_wait_status && !mysys_var->abort && + while (!m_wait_status && !thd_killed(thd) && wait_result != ETIMEDOUT && wait_result != ETIME) wait_result= mysql_cond_timedwait(&m_COND_wait_status, &m_LOCK_wait_status, abs_timeout); @@ -1048,14 +986,14 @@ MDL_wait::timed_wait(THD *thd, struct ti false, which means that the caller intends to restart the wait. */ - if (mysys_var->abort) + if (thd_killed(thd)) m_wait_status= KILLED; else if (set_status_on_timeout) m_wait_status= TIMEOUT; } result= m_wait_status; - MDL_EXIT_COND(thd, mysys_var, &m_LOCK_wait_status, old_msg); + thd_exit_cond(thd, old_msg); return result; } === modified file 'sql/mdl.h' --- a/sql/mdl.h 2010-07-13 18:01:54 +0000 +++ b/sql/mdl.h 2010-07-21 15:00:01 +0000 @@ -725,6 +725,10 @@ extern "C" const char *set_thd_proc_info const char *calling_function, const char *calling_file, const unsigned int calling_line); +extern "C" const char* thd_enter_cond(MYSQL_THD thd, mysql_cond_t *cond, + mysql_mutex_t *mutex, const char *msg); +extern "C" void thd_exit_cond(MYSQL_THD thd, const char *old_msg); + #ifndef DBUG_OFF extern mysql_mutex_t LOCK_open; #endif --===============0983399880829523923== 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-trunk-runtime-bug55223/ # testament_sha1: 218d4e5f0173e51771b329c68336e49cc12e5fcd # timestamp: 2010-07-21 17:00:06 +0200 # base_revision_id: jon.hauglid@stripped\ # xn93j8pwxa1lvca5 # # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWZqf6/MAAuL/gEAQAQB5d/// fv/+oL////BgB8dw7710CnXO2TxezAOvba97cjhJJBME9UwNJP0DUajyQ09Q0D1GYjU0ep6jZINC mmmpmTI0oekNANHqeoAAGjQAABKEaNQmmqfkT0p6Q0aeiZAAaAAAaDQJTSTFTyU9HkSbR6nlNT9R NDIxGAIyBgmEDjJk0YhiaYCBgTTBGCYmmmgAwgkUI1MGhGglPZDKap+k9TxU8hppPU8kyPUGQ9Ig qfmYtmSGh7pYfG8azBVBfOdw1h1WOiTXyshwZRsKJ7kXMlu0Vs+OJ9f5juR6b+3hL17DwN2VKZMM se8gnxVZCW/6L/GxLbOw2RDJCWj2hfONSYtUSKtjhEvhKGCzpLV362fFl/NyZ8WzbZvHqC6Azu20 lKLlVHax9/z0m7RvNO9VXjsaro0udWU+2LJE62zhK9JqXnBQhSLam6dl7u689LhTKJSHwvamhuMF XU9YfAnK2ct0ynLEFsSVHxla9EpqeciTDZLfdShUKjLyHlwsi3x80sVNOGV6KpTpJKrjbFQWqs0l mNJZ64zQYgXY6ih9vGl211bAvxXi5x+vlqpxVWB90UEGP0GjQYwfLyjqerrZumHKob5M5lVgrZqS kwoWkzlQFlTJWuijnJrpXokJ+pO6umYi1pq7crEYgmsQW8eEjge0qEa2jKyEgLQJIJ4Q4RZAZsgk 9ZvsDAroJxihxIkEA6TKEyEYG3vPeUSxeMTUg7mheVSNxZZhrAwlVngFluWyrNwIfE8QeP/LqF19 AMCRgwsttmexlXOzgYrfskWjkZFWmOr9zUqFssN4XNCYGh1FA6trNYwrZCkdOqBTWrOW2Asi67UR GcSGuwqrWE4g1qIvCqeZInMyIGwmdZHv4la+habC+dhC8MtcKXXaqFUys38i0qLg4KBbnDI2ngZy rxviBt0vIBrWSn0FTFMPZwtXRou1X7XzSAOQCQ6mWzVPMvxNj8p0EPmcSu/biHECVwHedRGYzcjc dIAd0tpcciAZFrGnVGNKwysKG6RZWA9bJOgyYrjv7IrrKqBbgYYTaVu0EmCwsZxU4WwYQeHgYygz DVIDeh0OaJbBxzLyomcDgYz02ZltlcXSJH1bIuaMoKNUQc3EpURSN7Q1K3X67xOYC3AzMsWsyqtk tREC/2LT+LU7MmZvCCqyb+V6LxGd/PJ/lZcHEUb+MJBtCgccz04sNs+gfoK2nyImRSEhdAunAYLh 2xHFA1+4U/prNRNXjeQrDcw+gzlcOBkc/iPz7A6cSREghqW37xBCybvhU+ToulhMEgZ2JF7D4H1X ucU+nzn4KgLu6kFZg1WFHAdp2cxSSo3C822YiGeccyKahjtc7sVyJpRti0YZRROoHyUy25EuyC+2 YK0JWl33crz+RD4dheb80fW1cadSaUexn4MK0jNTdLCPwMDgvUWI62lWfmwuvmNpJsktAfK6JO0A aLFFQzZYG8HenY1idVNSa1kJY0yi7PQ7iYQhGuJOhICwc068LTrpigKQtJhqkLh8eKbAYxtFpkWk bT5jjVpLn0T6C+o0J130uLbCVDTaSiEGEh5/KMSqqBBZQQ0ViWkEBFlEZ6gkBblAF+ZaLw0Re4kp FJB8cj0fE/BsPdIaWaeZUqjYeDQlLsIhFJQqz5NVbevSQHbPfl071i3nLK62TELcMkRWtgsygU0C Pae9nGok5M4Xy7IKOZKcjXBGFsPTChNyTWhREizEgj6ivvsBv5OBD7Dpy58DvgeHbImRJhauW4+h ia16zbAuTOw+ZLxYmxwXUmQhRKPIoV1oWJZocPA80+gDwEwrqCoE0I/qwG2xjxqSgeZImbiWiTNw zTtM6cv4+QHNEgUWtX/dhez+CuN/hwGwyPMDzEXBYQRb4W7a1fw5WH9unG6xFleTTZ4ofRniQDi1 lpYb+BzXeYr5ntTIa9ou5heeiO0Zrgj2FVPrXWmNaBrRKHmA131lvz9QSX2IvvBQDVaIkqCERWhR qqxliETUKbQo1QhdY2844zB6ljxsMz7RcjLlLJMs7idLyuM1IvGdZ7m3L9ydGpl+puyjZANyKlCQ jIOg7KKowhLdsptLCAqQgFGTmBlaz6czgc3fmcso8wHSPWVR1kBcWHRDhhHIDUf+jWzX+IJhWL3w grV6G2fpCXYCxRwJLW16/YxFdF6vATazwGks3zTo7LCgpNJRmcxMnfTsTGSccyXHMwKAUSwKGBeX ZEkyBAcAaIIDpnKIKNDfQokURZ6FBEJWOwRwFUJRYHks2MSvxiAGApwCWgYQg2lj2XudRQQSLk8P GBIlZVFsROc0mrG2VIjFbwpUtZLSuiJF4Ur9ZYUR94GJih5JztvGZJ5YIMtiTIScGNcGTWOkmGVa KL1GJk6yZVVXmSOUGSSaKJCBYTSkoqzJV0MiwqT1Ch2rYBzTGwzGjyjASwZ7S+eF+wrms4FzFPJE 1EcNtkQtsarMTeV/z7VWhO4PVUXz2qatb6zmR4HUuCzU5IdQ6kxHcMsO5VnknozamlsWYe47R0Sr ACYCZ/i7kinChITU/1+Y --===============0983399880829523923==--