From e6c6b183c8e0d12ae270069e69586318387e6a97 Mon Sep 17 00:00:00 2001 From: Nicholas Othieno Date: Wed, 13 Dec 2023 11:25:21 -0500 Subject: [PATCH] Fix crashing innodb due to innobase_share usage_count not being reduced for temp table accesses The MySQL engine is designed with the assumption that accesses to temporary tables will only ever occur from the thread that created the temporary table. Cross thread accesses of temporary tables are not expected and are therefore not designed for. However, if the information schema is queried for details about a temporary table, then the temporary table gets accessed from a thread that is different from the thread that created the temporary table, breaking the design assumption. When this cross-thread accesss of a temporary table happens for engines that use the InnoDB storage engine, the INNOBASE_SHARE use_count for the table gets incremented, but never gets decremented when the none-owning thread completes. In certain situations, this can lead to a crash of the database engine, when invariants in the function innobase_build_index_translation fail on assertion. For example we found a use case that cause MySQL to crash with the assertion failure below: (gdb) bt #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 #1 0x00007f3132141859 in __GI_abort () at abort.c:79 #2 0x000055ed7452a19a in ut_dbg_assertion_failed (expr=0x55ed7489f848 "share->idx_trans_tbl.index_count == mysql_num_index", file=0x55ed7489ea90 "/github/MySQL5.7/mysql-server/storage/innobase/handler/ha_innodb.cc", line=5680) at /github/MySQL5.7/mysql-server/storage/innobase/ut/ut0dbg.cc:75 #3 0x000055ed7437ecda in innobase_build_index_translation (table=0x7f30bcaaa4d0, ib_table=0x7f30bc023888, share=0x7f30b002c100) at /github/MySQL5.7/mysql-server/storage/innobase/handler/ha_innodb.cc:5680 #4 0x000055ed7437fe6d in ha_innobase::open (this=0x7f30bcaa6a10, name=0x7f30bcaab1c0 "./test/#sql-bae1d_3", mode=2, test_if_locked=2) at /github/MySQL5.7/mysql-server/storage/innobase/handler/ha_innodb.cc:6115 The above issue is fixed by allowing the use_count for an INNODB_SHARE object of a temporary table to be decremented during query cleanup, whenever a temporary table is accessed from a none-owning thread. Care is taken to make sure that the use_count decrement is done inside of the a critical region protected by the innobase_share_mutex mutex. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc. --- sql/handler.h | 10 ++++++++++ sql/sql_base.cc | 10 ++++++++++ sql/table.cc | 5 +++++ sql/table.h | 1 + storage/innobase/handler/ha_innodb.cc | 23 +++++++++++++++++++++++ storage/innobase/handler/ha_innodb.h | 1 + 6 files changed, 50 insertions(+) diff --git a/sql/handler.h b/sql/handler.h index 285a8bcbb3d2..eb1d9038afdf 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -2816,6 +2816,16 @@ class handler :public Sql_alloc double estimate_in_memory_buffer(ulonglong table_index_size) const; public: + /* Reduce the reference count for the storage in engine share + This member has been created to target the INNODB storage + engine, for a case where a thread other than the owning + thread of a temporary table accesses the temporary + table. In INNODB the reference count for the INNOBASE_SHARE + is increased when this access happens. This function is + provided to allow MySQL to reduce this reference count once + the access is complete + */ + virtual void reduce_share_refcount() { return; } virtual ha_rows multi_range_read_info_const(uint keyno, RANGE_SEQ_IF *seq, void *seq_init_param, uint n_ranges, uint *bufsz, diff --git a/sql/sql_base.cc b/sql/sql_base.cc index d5f000f45d6a..fde0dc0824f9 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -1612,6 +1612,16 @@ void close_thread_tables(THD *thd) table->file->extra(HA_EXTRA_DETACH_CHILDREN); table->cleanup_gc_items(); } + + char tmp_table_name_prefix[] = "#sql-"; + const size_t prefix_len = 5; + /* Check if the table is a temp table owned by a different thread */ + if (table->query_id != thd->query_id && !memcmp(tmp_table_name_prefix, table->s->table_name.str, prefix_len)) + { + sql_print_information("table: '%s' owning_query_id: %lu but the query id for this thread is %lu", + table->s->table_name.str, (ulong) table->query_id, (ulong) thd->query_id); + reduce_se_share_ref_count(table); + } } /* diff --git a/sql/table.cc b/sql/table.cc index d469b12a3015..542b835f22f3 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -3143,6 +3143,11 @@ static bool unpack_gcol_info_from_frm(THD *thd, DBUG_RETURN(TRUE); } +void reduce_se_share_ref_count(TABLE *table_arg) +{ + table_arg->file->reduce_share_refcount(); +} + /* Read data from a binary .frm file from MySQL 3.23 - 5.0 into TABLE_SHARE */ diff --git a/sql/table.h b/sql/table.h index 949508cd87b2..f2cc424025f8 100644 --- a/sql/table.h +++ b/sql/table.h @@ -2974,6 +2974,7 @@ void init_tmp_table_share(THD *thd, TABLE_SHARE *share, const char *key, size_t key_length, const char *table_name, const char *path); void free_table_share(TABLE_SHARE *share); +void reduce_se_share_ref_count(TABLE *table_arg); /** diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index f87bb25e50e1..33d015b38701 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -6992,6 +6992,29 @@ ha_innobase::innobase_initialize_autoinc() dict_table_autoinc_initialize(m_prebuilt->table, auto_inc); } +/******************************************* + * Reduced the reference count for the innobase_share + * This function is ONLY applicable when a thread that does not + * own a temporary table accesses the temporary table. This + * access causes the reference count of the temporary table to + * be increased. INNODB was originally written with the + * assumption that a none owning thread will not accesss a + * temporary table owned by another thread. But use cases + * have been observed that cause this access to happen. + * This increase of the refcount by the none-owning thread + * can cause the engine to crash if the innobase share is + * recycled to be used to access a differrent table from + * within the owning thread + */ +void ha_innobase::reduce_share_refcount(){ + mysql_mutex_lock(&innobase_share_mutex); + if (m_share->use_count > 0) + { + --m_share->use_count; + } + mysql_mutex_unlock(&innobase_share_mutex); +} + /*****************************************************************//** Creates and opens a handle to a table which already exists in an InnoDB database. diff --git a/storage/innobase/handler/ha_innodb.h b/storage/innobase/handler/ha_innodb.h index 43c7a835bdd5..a4e0ae6c5181 100644 --- a/storage/innobase/handler/ha_innodb.h +++ b/storage/innobase/handler/ha_innodb.h @@ -108,6 +108,7 @@ class ha_innobase: public handler const key_map* keys_to_use_for_scanning(); int open(const char *name, int mode, uint test_if_locked); + void reduce_share_refcount(); /** Opens dictionary table object using table name. For partition, we need to try alternative lower/upper case names to support moving data files across