revision-id: cf9bafc8cb0943c914d118525888140ea15e6b55 (mariadb-10.0.36-50-gcf9bafc) parent(s): 8e716138cef2d9be603cdf71701d82bcb72dfd69 author: Sergey Vojtovich committer: Sergey Vojtovich timestamp: 2018-10-18 23:50:50 +0400 message: MDEV-14815 - Server crash or AddressSanitizer errors or valgrind warnings in thr_lock / has_old_lock upon FLUSH TABLES Explicit partition access of partitioned MEMORY table under LOCK TABLES may cause subsequent statements to crash the server, deadlock, trigger valgrind warnings or ASAN errors. Freed memory was being used due to incorrect cleanup. At least MyISAM and InnoDB don't seem to be affected, since their THR_LOCK structures don't survive FLUSH TABLES. MEMORY keeps table shared data (including THR_LOCK) even if there're no open instances. There's partition_info::lock_partitions bitmap, which holds bits of partitions allowed to be accessed after pruning. This bitmap is updated for each individual statement. This bitmap was abused in ha_partition::store_lock() such that when we need to unlock a table, locked by LOCK TABLES, only locks for partitions that were accessed by previous statement were released. Eventually FLUSH TABLES frees THR_LOCK_DATA objects, which are still linked into THR_LOCK lists. When such THR_LOCK gets reused we end up with freed memory access. Fixed by using ha_partition::m_locked_partitions bitmap similarly to ha_partition::external_lock(). Some unused code removed. --- mysql-test/r/partition_explicit_prune.result | 19 ++++++++ mysql-test/t/partition_explicit_prune.test | 19 ++++++++ sql/ha_partition.cc | 9 +++- sql/ha_partition.h | 1 - sql/lock.cc | 17 ------- sql/lock.h | 1 - sql/sql_base.cc | 70 ---------------------------- sql/sql_insert.cc | 2 +- 8 files changed, 46 insertions(+), 92 deletions(-) diff --git a/mysql-test/r/partition_explicit_prune.result b/mysql-test/r/partition_explicit_prune.result index 3ca1e68..c8ab243 100644 --- a/mysql-test/r/partition_explicit_prune.result +++ b/mysql-test/r/partition_explicit_prune.result @@ -1870,3 +1870,22 @@ CREATE TABLE t2 LIKE t1 PARTITION (p0, p2); ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'PARTITION (p0, p2)' at line 1 DROP TABLE t1; SET @@default_storage_engine = @old_default_storage_engine; +# +# MDEV-14815 - Server crash or AddressSanitizer errors or valgrind warnings in thr_lock / has_old_lock upon FLUSH TABLES +# +CREATE TABLE t1 (i INT) ENGINE=MEMORY PARTITION BY RANGE (i) (PARTITION p0 VALUES LESS THAN (4), PARTITION pm VALUES LESS THAN MAXVALUE); +CREATE TABLE t2 (i INT) ENGINE=MEMORY; +LOCK TABLE t1 WRITE, t2 WRITE; +SELECT * FROM t1 PARTITION (p0); +i +FLUSH TABLES; +SELECT * FROM t1 PARTITION (p0); +i +ALTER TABLE t1 TRUNCATE PARTITION p0; +SELECT * FROM t1 PARTITION (p0); +i +ALTER TABLE t1 EXCHANGE PARTITION p0 WITH TABLE t2; +SELECT * FROM t1 PARTITION (p0); +i +UNLOCK TABLES; +DROP TABLE t1, t2; diff --git a/mysql-test/t/partition_explicit_prune.test b/mysql-test/t/partition_explicit_prune.test index 68b829f..b8b6e48 100644 --- a/mysql-test/t/partition_explicit_prune.test +++ b/mysql-test/t/partition_explicit_prune.test @@ -858,3 +858,22 @@ CREATE TABLE t2 LIKE t1 PARTITION (p0, p2); DROP TABLE t1; SET @@default_storage_engine = @old_default_storage_engine; + + +--echo # +--echo # MDEV-14815 - Server crash or AddressSanitizer errors or valgrind warnings in thr_lock / has_old_lock upon FLUSH TABLES +--echo # +CREATE TABLE t1 (i INT) ENGINE=MEMORY PARTITION BY RANGE (i) (PARTITION p0 VALUES LESS THAN (4), PARTITION pm VALUES LESS THAN MAXVALUE); +CREATE TABLE t2 (i INT) ENGINE=MEMORY; +LOCK TABLE t1 WRITE, t2 WRITE; +SELECT * FROM t1 PARTITION (p0); +FLUSH TABLES; +SELECT * FROM t1 PARTITION (p0); +ALTER TABLE t1 TRUNCATE PARTITION p0; +SELECT * FROM t1 PARTITION (p0); +ALTER TABLE t1 EXCHANGE PARTITION p0 WITH TABLE t2; +SELECT * FROM t1 PARTITION (p0); +UNLOCK TABLES; + +# Cleanup +DROP TABLE t1, t2; diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc index 0488ebf..02de92a 100644 --- a/sql/ha_partition.cc +++ b/sql/ha_partition.cc @@ -3907,9 +3907,14 @@ THR_LOCK_DATA **ha_partition::store_lock(THD *thd, } else { - for (i= bitmap_get_first_set(&(m_part_info->lock_partitions)); + MY_BITMAP *used_partitions= lock_type == TL_UNLOCK || + lock_type == TL_IGNORE ? + &m_locked_partitions : + &m_part_info->lock_partitions; + + for (i= bitmap_get_first_set(used_partitions); i < m_tot_parts; - i= bitmap_get_next_set(&m_part_info->lock_partitions, i)) + i= bitmap_get_next_set(used_partitions, i)) { DBUG_PRINT("info", ("store lock %d iteration", i)); to= m_file[i]->store_lock(thd, to, lock_type); diff --git a/sql/ha_partition.h b/sql/ha_partition.h index 71ae84b..0e9f5c4 100644 --- a/sql/ha_partition.h +++ b/sql/ha_partition.h @@ -251,7 +251,6 @@ class ha_partition :public handler /* Variables for lock structures. */ - THR_LOCK_DATA lock; /* MySQL lock */ bool auto_increment_lock; /**< lock reading/updating auto_inc */ /** diff --git a/sql/lock.cc b/sql/lock.cc index 3354da2..09e8dbd 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -541,23 +541,6 @@ void mysql_lock_remove(THD *thd, MYSQL_LOCK *locked,TABLE *table) } -/** Abort all other threads waiting to get lock in table. */ - -void mysql_lock_abort(THD *thd, TABLE *table, bool upgrade_lock) -{ - MYSQL_LOCK *locked; - DBUG_ENTER("mysql_lock_abort"); - - if ((locked= get_lock_data(thd, &table, 1, GET_LOCK_UNLOCK))) - { - for (uint i=0; i < locked->lock_count; i++) - thr_abort_locks(locked->locks[i]->lock, upgrade_lock); - my_free(locked); - } - DBUG_VOID_RETURN; -} - - /** Abort one thread / table combination. diff --git a/sql/lock.h b/sql/lock.h index a4833cd..a9183bf 100644 --- a/sql/lock.h +++ b/sql/lock.h @@ -32,7 +32,6 @@ void mysql_unlock_tables(THD *thd, MYSQL_LOCK *sql_lock, bool free_lock= 1); void mysql_unlock_read_tables(THD *thd, MYSQL_LOCK *sql_lock); void mysql_unlock_some_tables(THD *thd, TABLE **table,uint count); void mysql_lock_remove(THD *thd, MYSQL_LOCK *locked,TABLE *table); -void mysql_lock_abort(THD *thd, TABLE *table, bool upgrade_lock); bool mysql_lock_abort_for_thread(THD *thd, TABLE *table); MYSQL_LOCK *mysql_lock_merge(MYSQL_LOCK *a,MYSQL_LOCK *b); /* Lock based on name */ diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 1a17c8e..403ba85 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -9100,76 +9100,6 @@ my_bool mysql_rm_tmp_tables(void) unireg support functions *****************************************************************************/ -/** - A callback to the server internals that is used to address - special cases of the locking protocol. - Invoked when acquiring an exclusive lock, for each thread that - has a conflicting shared metadata lock. - - This function: - - aborts waiting of the thread on a data lock, to make it notice - the pending exclusive lock and back off. - - if the thread is an INSERT DELAYED thread, sends it a KILL - signal to terminate it. - - @note This function does not wait for the thread to give away its - locks. Waiting is done outside for all threads at once. - - @param thd Current thread context - @param in_use The thread to wake up - @param needs_thr_lock_abort Indicates that to wake up thread - this call needs to abort its waiting - on table-level lock. - - @retval TRUE if the thread was woken up - @retval FALSE otherwise. - - @note It is one of two places where border between MDL and the - rest of the server is broken. -*/ - -bool mysql_notify_thread_having_shared_lock(THD *thd, THD *in_use, - bool needs_thr_lock_abort) -{ - bool signalled= FALSE; - if ((in_use->system_thread & SYSTEM_THREAD_DELAYED_INSERT) && - !in_use->killed) - { - in_use->killed= KILL_SYSTEM_THREAD; - mysql_mutex_lock(&in_use->mysys_var->mutex); - if (in_use->mysys_var->current_cond) - { - mysql_mutex_lock(in_use->mysys_var->current_mutex); - mysql_cond_broadcast(in_use->mysys_var->current_cond); - mysql_mutex_unlock(in_use->mysys_var->current_mutex); - } - mysql_mutex_unlock(&in_use->mysys_var->mutex); - signalled= TRUE; - } - - if (needs_thr_lock_abort) - { - mysql_mutex_lock(&in_use->LOCK_thd_data); - for (TABLE *thd_table= in_use->open_tables; - thd_table ; - thd_table= thd_table->next) - { - /* - Check for TABLE::needs_reopen() is needed since in some places we call - handler::close() for table instance (and set TABLE::db_stat to 0) - and do not remove such instances from the THD::open_tables - for some time, during which other thread can see those instances - (e.g. see partitioning code). - */ - if (!thd_table->needs_reopen()) - signalled|= mysql_lock_abort_for_thread(thd, thd_table); - } - mysql_mutex_unlock(&in_use->LOCK_thd_data); - } - return signalled; -} - - int setup_ftfuncs(SELECT_LEX *select_lex) { List_iterator<Item_func_match> li(*(select_lex->ftfunc_list)), diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 8b03c72..5ecdd72 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -2353,7 +2353,7 @@ TABLE *Delayed_insert::get_local_table(THD* client_thd) The thread could be killed with an error message if di->handle_inserts() or di->open_and_lock_table() fails. The thread could be killed without an error message if - killed using mysql_notify_thread_having_shared_lock() or + killed using THD::notify_shared_lock() or kill_delayed_threads_for_table(). */ if (!thd.is_error())