From 7f498fbab888bd24d76a0292f4d22cacfb2f95b2 Mon Sep 17 00:00:00 2001 From: Kristian Nielsen Date: Fri, 15 Mar 2024 00:41:28 +0100 Subject: [PATCH 1/4] Fix "Assertion `THR_PFS_initialized' failed" in main.bootstrap This patch makes the server wait for the manager thread to actually start before proceeding with server startup. Without this, if thread scheduling is really slow and the server shutdowns quickly, then it is possible that the manager thread is not yet started when shutdown_performance_schema() is called. If the manager thread starts at just the wrong moment and just before the main server reaches exit(), the thread can try to access no longer available performance schema data. This was seen as occasional assertion in the main.bootstrap test. As an additional improvement, make sure to run all pending actions before exiting the manager thread. Reviewed-by: Monty Signed-off-by: Kristian Nielsen --- sql/sql_manager.cc | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/sql/sql_manager.cc b/sql/sql_manager.cc index 3d3728b9e00..5cd66d8047a 100644 --- a/sql/sql_manager.cc +++ b/sql/sql_manager.cc @@ -76,7 +76,9 @@ pthread_handler_t handle_manager(void *arg __attribute__((unused))) pthread_detach_this_thread(); manager_thread = pthread_self(); mysql_mutex_lock(&LOCK_manager); - while (!abort_manager) + manager_thread_in_use = 1; + mysql_cond_signal(&COND_manager); + while (!abort_manager || cb_list) { /* XXX: This will need to be made more general to handle different * polling needs. */ @@ -116,6 +118,7 @@ pthread_handler_t handle_manager(void *arg __attribute__((unused))) } mysql_mutex_lock(&LOCK_manager); } + DBUG_ASSERT(cb_list == NULL); manager_thread_in_use = 0; mysql_mutex_unlock(&LOCK_manager); mysql_mutex_destroy(&LOCK_manager); @@ -135,12 +138,19 @@ void start_handle_manager() pthread_t hThread; int err; DBUG_EXECUTE_IF("delay_start_handle_manager", my_sleep(1000);); - manager_thread_in_use = 1; mysql_cond_init(key_COND_manager, &COND_manager,NULL); mysql_mutex_init(key_LOCK_manager, &LOCK_manager, NULL); if ((err= mysql_thread_create(key_thread_handle_manager, &hThread, &connection_attrib, handle_manager, 0))) + { sql_print_warning("Can't create handle_manager thread (errno: %M)", err); + DBUG_VOID_RETURN; + } + + mysql_mutex_lock(&LOCK_manager); + while (!manager_thread_in_use) + mysql_cond_wait(&COND_manager, &LOCK_manager); + mysql_mutex_unlock(&LOCK_manager); } DBUG_VOID_RETURN; } From fb774eb1ebab7d397c10a516be364d395440d079 Mon Sep 17 00:00:00 2001 From: Kristian Nielsen Date: Fri, 15 Mar 2024 14:54:36 +0100 Subject: [PATCH 2/4] Fix occasional test failure of rpl.rpl_parallel_stop_slave Signed-off-by: Kristian Nielsen --- .../rpl_parallel_stop_slave.result | 4 +++- .../rpl/r/rpl_parallel_stop_slave.result | 4 +++- .../suite/rpl/t/rpl_parallel_stop_slave.test | 23 ++++++++++++++++++- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/mysql-test/suite/binlog_encryption/rpl_parallel_stop_slave.result b/mysql-test/suite/binlog_encryption/rpl_parallel_stop_slave.result index 6c9fd168e73..de4798f0cee 100644 --- a/mysql-test/suite/binlog_encryption/rpl_parallel_stop_slave.result +++ b/mysql-test/suite/binlog_encryption/rpl_parallel_stop_slave.result @@ -35,7 +35,9 @@ connection con_temp1; BEGIN; INSERT INTO t2 VALUES (21); connection server_2; -START SLAVE; +START SLAVE IO_THREAD; +include/wait_for_slave_param.inc [Read_Master_Log_Pos] +START SLAVE SQL_THREAD; connection con_temp2; SET @old_dbug= @@GLOBAL.debug_dbug; SET GLOBAL debug_dbug="+d,rpl_parallel_wait_for_done_trigger"; diff --git a/mysql-test/suite/rpl/r/rpl_parallel_stop_slave.result b/mysql-test/suite/rpl/r/rpl_parallel_stop_slave.result index 6c9fd168e73..de4798f0cee 100644 --- a/mysql-test/suite/rpl/r/rpl_parallel_stop_slave.result +++ b/mysql-test/suite/rpl/r/rpl_parallel_stop_slave.result @@ -35,7 +35,9 @@ connection con_temp1; BEGIN; INSERT INTO t2 VALUES (21); connection server_2; -START SLAVE; +START SLAVE IO_THREAD; +include/wait_for_slave_param.inc [Read_Master_Log_Pos] +START SLAVE SQL_THREAD; connection con_temp2; SET @old_dbug= @@GLOBAL.debug_dbug; SET GLOBAL debug_dbug="+d,rpl_parallel_wait_for_done_trigger"; diff --git a/mysql-test/suite/rpl/t/rpl_parallel_stop_slave.test b/mysql-test/suite/rpl/t/rpl_parallel_stop_slave.test index 4eeddc927e0..d6da88c3d6c 100644 --- a/mysql-test/suite/rpl/t/rpl_parallel_stop_slave.test +++ b/mysql-test/suite/rpl/t/rpl_parallel_stop_slave.test @@ -53,6 +53,7 @@ COMMIT; INSERT INTO t3 VALUES(21, 21); INSERT INTO t3 VALUES(22, 22); --save_master_pos +--let $master_pos= query_get_value(SHOW MASTER STATUS, Position, 1) # Start a connection that will block the replicated transaction halfway. --connection con_temp1 @@ -60,7 +61,27 @@ BEGIN; INSERT INTO t2 VALUES (21); --connection server_2 -START SLAVE; + +# +# Parallel replication will complete any in-progress event group at STOP SLAVE, +# but only if the event group is already queued up for the worker thread. If +# the SQL driver thread is delayed in queueing up events, the parallel worker +# thread can abort the event group, leaving the non-transactional update to the +# MyISAM table that cannot be rolled back (MDEV-7432). If this happens the test +# would fail with duplicate key error after slave restart. +# +# To avoid this, we here wait for the IO thread to read all master events, and +# for the SQL driver thread to queue all the events for workers. This wait +# should be removed if/when MDEV-7432 is fixed. +# +START SLAVE IO_THREAD; +--let $slave_param= Read_Master_Log_Pos +--let $slave_param_value= $master_pos +--source include/wait_for_slave_param.inc +START SLAVE SQL_THREAD; +--let $wait_condition= SELECT COUNT(*)=1 FROM INFORMATION_SCHEMA.PROCESSLIST WHERE State LIKE '%Slave has read all relay log; waiting for more updates%' +--source include/wait_condition.inc + # Wait for the MyISAM change to be visible, after which replication will wait # for con_temp1 to roll back. --let $wait_condition= SELECT COUNT(*) = 1 FROM t1 WHERE a=20 From 09d991d01c47e22030879e5bf0c7a4893a598199 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 18 Mar 2024 16:01:29 +0200 Subject: [PATCH 3/4] MDEV-33478: Tests massively fail with clang-18 -fsanitize=memory Starting with clang-16, MemorySanitizer appears to check that uninitialized values not be passed by value nor returned. Previously, it was allowed to copy uninitialized data in such cases. get_foreign_key_info(): Remove a local variable that was passed uninitialized to a function. DsMrr_impl: Initialize key_buffer, because DsMrr_impl::dsmrr_init() is reading it. test_bind_result_ext1(): MYSQL_TYPE_LONG is 32 bits, hence we must use a 32-bit type, such as int. sizeof(long) differs between LP64 and LLP64 targets. --- sql/debug_sync.cc | 2 +- sql/item_func.h | 2 +- sql/log.h | 11 +++++++++++ sql/multi_range_read.h | 10 +++------- sql/sql_lex.h | 2 +- storage/innobase/handler/ha_innodb.cc | 11 ++++------- storage/myisam/ft_boolean_search.c | 2 ++ tests/mysql_client_test.c | 4 ++-- 8 files changed, 25 insertions(+), 19 deletions(-) diff --git a/sql/debug_sync.cc b/sql/debug_sync.cc index eec95bee1f2..160851ae004 100644 --- a/sql/debug_sync.cc +++ b/sql/debug_sync.cc @@ -1008,7 +1008,7 @@ static bool debug_sync_eval_action(THD *thd, char *action_str, char *action_end) st_debug_sync_action *action= NULL; const char *errmsg; char *ptr; - char *token; + char *token= nullptr; uint token_length= 0; DBUG_ENTER("debug_sync_eval_action"); DBUG_ASSERT(thd); diff --git a/sql/item_func.h b/sql/item_func.h index 0f4f8928847..13b522dc25c 100644 --- a/sql/item_func.h +++ b/sql/item_func.h @@ -90,7 +90,7 @@ public: static void wrong_param_count_error(const LEX_CSTRING &schema_name, const LEX_CSTRING &func_name); - table_map not_null_tables_cache; + table_map not_null_tables_cache= 0; enum Functype { UNKNOWN_FUNC,EQ_FUNC,EQUAL_FUNC,NE_FUNC,LT_FUNC,LE_FUNC, GE_FUNC,GT_FUNC,FT_FUNC, diff --git a/sql/log.h b/sql/log.h index fbe0f30f22b..dd31081c404 100644 --- a/sql/log.h +++ b/sql/log.h @@ -426,6 +426,7 @@ struct wait_for_commit; class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG { +#ifdef HAVE_PSI_INTERFACE /** The instrumentation key to use for @ LOCK_index. */ PSI_mutex_key m_key_LOCK_index; /** The instrumentation key to use for @ COND_relay_log_updated */ @@ -440,6 +441,16 @@ class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG PSI_cond_key m_key_COND_queue_busy; /** The instrumentation key to use for LOCK_binlog_end_pos. */ PSI_mutex_key m_key_LOCK_binlog_end_pos; +#else + static constexpr PSI_mutex_key m_key_LOCK_index= 0; + static constexpr PSI_cond_key m_key_relay_log_update= 0; + static constexpr PSI_cond_key m_key_bin_log_update= 0; + static constexpr PSI_file_key m_key_file_log= 0, m_key_file_log_cache= 0; + static constexpr PSI_file_key m_key_file_log_index= 0; + static constexpr PSI_file_key m_key_file_log_index_cache= 0; + static constexpr PSI_cond_key m_key_COND_queue_busy= 0; + static constexpr PSI_mutex_key m_key_LOCK_binlog_end_pos= 0; +#endif struct group_commit_entry { diff --git a/sql/multi_range_read.h b/sql/multi_range_read.h index 57cfd21727f..5e7879d9226 100644 --- a/sql/multi_range_read.h +++ b/sql/multi_range_read.h @@ -556,10 +556,6 @@ class DsMrr_impl public: typedef void (handler::*range_check_toggle_func_t)(bool on); - DsMrr_impl() - : secondary_file(NULL), - rowid_filter(NULL) {}; - void init(handler *h_arg, TABLE *table_arg) { primary_file= h_arg; @@ -581,7 +577,7 @@ public: int dsmrr_explain_info(uint mrr_mode, char *str, size_t size); private: /* Buffer to store (key, range_id) pairs */ - Lifo_buffer *key_buffer; + Lifo_buffer *key_buffer= nullptr; /* The "owner" handler object (the one that is expected to "own" this object @@ -594,13 +590,13 @@ private: Secondary handler object. (created when needed, we need it when we need to run both index scan and rnd_pos() scan at the same time) */ - handler *secondary_file; + handler *secondary_file= nullptr; /* The rowid filter that DS-MRR has "unpushed" from the storage engine. If it's present, DS-MRR will use it. */ - Rowid_filter *rowid_filter; + Rowid_filter *rowid_filter= nullptr; uint keyno; /* index we're running the scan on */ /* TRUE <=> need range association, buffers hold {rowid, range_id} pairs */ diff --git a/sql/sql_lex.h b/sql/sql_lex.h index b6d91896142..3ab50d4aaa8 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -3260,7 +3260,7 @@ public: Table_type table_type; /* Used for SHOW CREATE */ List ref_list; List users_list; - List *insert_list,field_list,value_list,update_list; + List *insert_list= nullptr,field_list,value_list,update_list; List many_values; List var_list; List stmt_var_list; //SET_STATEMENT values diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index ad2e022f0b3..19cfa21c84e 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -12108,7 +12108,7 @@ create_table_info_t::create_foreign_keys() dict_index_t* index = NULL; fkerr_t index_error = FK_SUCCESS; dict_index_t* err_index = NULL; - ulint err_col; + ulint err_col = 0; const bool tmp_table = m_flags2 & DICT_TF2_TEMPORARY; const CHARSET_INFO* cs = thd_charset(m_thd); const char* operation = "Create "; @@ -15150,7 +15150,6 @@ get_foreign_key_info( char tmp_buff[NAME_LEN+1]; char name_buff[NAME_LEN+1]; const char* ptr; - LEX_CSTRING* referenced_key_name; LEX_CSTRING* name = NULL; if (dict_table_t::is_temporary_name(foreign->foreign_table_name)) { @@ -15255,18 +15254,16 @@ get_foreign_key_info( if (foreign->referenced_index && foreign->referenced_index->name != NULL) { - referenced_key_name = thd_make_lex_string( + f_key_info.referenced_key_name = thd_make_lex_string( thd, - f_key_info.referenced_key_name, + nullptr, foreign->referenced_index->name, strlen(foreign->referenced_index->name), 1); } else { - referenced_key_name = NULL; + f_key_info.referenced_key_name = NULL; } - f_key_info.referenced_key_name = referenced_key_name; - pf_key_info = (FOREIGN_KEY_INFO*) thd_memdup(thd, &f_key_info, sizeof(FOREIGN_KEY_INFO)); diff --git a/storage/myisam/ft_boolean_search.c b/storage/myisam/ft_boolean_search.c index f69f1869383..3d95fffacaf 100644 --- a/storage/myisam/ft_boolean_search.c +++ b/storage/myisam/ft_boolean_search.c @@ -287,6 +287,8 @@ static int ftb_parse_query_internal(MYSQL_FTPARSER_PARAM *param, uchar *end= (uchar*) query + len; FT_WORD w; + w.pos= NULL; + w.len= 0; info.prev= ' '; info.quot= 0; while (ft_get_word(cs, start, end, &w, &info)) diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c index fde6148f4e0..b1e40ea5c88 100644 --- a/tests/mysql_client_test.c +++ b/tests/mysql_client_test.c @@ -3842,7 +3842,7 @@ static void test_bind_result_ext1() short i_data; uchar b_data; int f_data; - long bData; + int bData; char d_data[20]; double szData; MYSQL_BIND my_bind[8]; @@ -3938,7 +3938,7 @@ static void test_bind_result_ext1() fprintf(stdout, "\n data (float) : %d(%lu)", f_data, length[4]); fprintf(stdout, "\n data (double) : %s(%lu)", d_data, length[5]); - fprintf(stdout, "\n data (bin) : %ld(%lu)", bData, length[6]); + fprintf(stdout, "\n data (bin) : %d(%lu)", bData, length[6]); fprintf(stdout, "\n data (str) : %g(%lu)", szData, length[7]); } From 4592af2e84dfddeb8370c0c8c3cb2d35dddc32db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 18 Mar 2024 16:01:58 +0200 Subject: [PATCH 4/4] Work around missing MSAN instrumentation Let us skip the recently added test main.mysql-interactive if an instrumented ncurses library is not available. In InnoDB, let us work around an uninstrumented libnuma, by declaring that the objects returned by numa_get_mems_allowed() are initialized. --- mysql-test/main/mysql-interactive.test | 2 ++ storage/innobase/buf/buf0buf.cc | 3 +++ 2 files changed, 5 insertions(+) diff --git a/mysql-test/main/mysql-interactive.test b/mysql-test/main/mysql-interactive.test index 2015e9d667d..0051d8e58c6 100644 --- a/mysql-test/main/mysql-interactive.test +++ b/mysql-test/main/mysql-interactive.test @@ -3,6 +3,8 @@ --echo # source include/not_embedded.inc; source include/not_windows.inc; +# this would need an instrumented ncurses library +source include/not_msan.inc; error 0,1; exec $MYSQL -V|grep -q readline; diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/buf0buf.cc index c0dece69cf8..f37a96d5a7c 100644 --- a/storage/innobase/buf/buf0buf.cc +++ b/storage/innobase/buf/buf0buf.cc @@ -82,6 +82,8 @@ struct set_numa_interleave_t if (srv_numa_interleave) { struct bitmask *numa_mems_allowed = numa_get_mems_allowed(); + MEM_MAKE_DEFINED(numa_mems_allowed, + sizeof *numa_mems_allowed); ib::info() << "Setting NUMA memory policy to" " MPOL_INTERLEAVE"; if (set_mempolicy(MPOL_INTERLEAVE, @@ -1072,6 +1074,7 @@ inline bool buf_pool_t::chunk_t::create(size_t bytes) if (srv_numa_interleave) { struct bitmask *numa_mems_allowed= numa_get_mems_allowed(); + MEM_MAKE_DEFINED(numa_mems_allowed, sizeof *numa_mems_allowed); if (mbind(mem, mem_size(), MPOL_INTERLEAVE, numa_mems_allowed->maskp, numa_mems_allowed->size, MPOL_MF_MOVE))