From aae9b50a53463d1ebfbf7b22cd78eda097536b46 Mon Sep 17 00:00:00 2001 From: Monty Date: Mon, 20 Jan 2025 19:30:19 +0200 Subject: [PATCH] Added VALGRIND_YIELD to be able to abort from busy loops Valgrind is single threaded and only changes threads as part of system calls or waits. I found some busy loops where the server assumes that some other thread will change the state, which will not happen with valgrind. Added VALGRIND_YIELD to the loops, which calls pthread_yield() if HAVE_VALGRIND is defined. Added pthread_yield() to the loops in table_cache. We should consider changing some of the VALGRIND_YIELD calls to call pthread_yield() as busy loop without any sleep() is usually a bad thing. Reviewer: svojtovich@gmail.com --- include/my_valgrind.h | 4 ++++ sql/sql_base.cc | 2 ++ sql/table_cache.cc | 1 + storage/innobase/trx/trx0purge.cc | 4 ++++ 4 files changed, 11 insertions(+) diff --git a/include/my_valgrind.h b/include/my_valgrind.h index dfe2c3db7b3..a99deff568b 100644 --- a/include/my_valgrind.h +++ b/include/my_valgrind.h @@ -42,6 +42,7 @@ # else # define MSAN_STAT_WORKAROUND(st) ((void) 0) # endif +# define VALGRIND_YIELD #elif defined(HAVE_VALGRIND_MEMCHECK_H) && defined(HAVE_valgrind) # include # define HAVE_MEM_CHECK @@ -55,6 +56,7 @@ # define MEM_SET_VBITS(a,b,len) VALGRIND_SET_VBITS(a,b,len) # define REDZONE_SIZE 8 # define MSAN_STAT_WORKAROUND(st) ((void) 0) +# define VALGRIND_YIELD pthread_yield() #elif defined(__SANITIZE_ADDRESS__) && (!defined(_MSC_VER) || defined (__clang__)) # include /* How to do manual poisoning: @@ -70,6 +72,7 @@ https://github.com/google/sanitizers/wiki/AddressSanitizerManualPoisoning */ # define MEM_SET_VBITS(a,b,len) ((void) 0) # define MSAN_STAT_WORKAROUND(st) ((void) 0) # define REDZONE_SIZE 8 +# define VALGRIND_YIELD #else # define MEM_UNDEFINED(a,len) ((void) 0) # define MEM_MAKE_ADDRESSABLE(a,len) ((void) 0) @@ -81,6 +84,7 @@ https://github.com/google/sanitizers/wiki/AddressSanitizerManualPoisoning */ # define MEM_SET_VBITS(a,b,len) ((void) 0) # define REDZONE_SIZE 0 # define MSAN_STAT_WORKAROUND(st) ((void) 0) +# define VALGRIND_YIELD #endif /* __has_feature(memory_sanitizer) */ #ifdef TRASH_FREED_MEMORY diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 98a19e2cd83..bf01e309238 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -4721,6 +4721,7 @@ restart: goto error; error= FALSE; + pthread_yield(); goto restart; } goto error; @@ -4786,6 +4787,7 @@ restart: error= FALSE; sroutine_to_open= &thd->lex->sroutines_list.first; + pthread_yield(); goto restart; } /* diff --git a/sql/table_cache.cc b/sql/table_cache.cc index 8a569fd5adb..89790d37ab3 100644 --- a/sql/table_cache.cc +++ b/sql/table_cache.cc @@ -901,6 +901,7 @@ retry: { mysql_mutex_unlock(&element->LOCK_table_share); lf_hash_search_unpin(thd->tdc_hash_pins); + VALGRIND_YIELD; goto retry; } lf_hash_search_unpin(thd->tdc_hash_pins); diff --git a/storage/innobase/trx/trx0purge.cc b/storage/innobase/trx/trx0purge.cc index 049de3f230f..4878aea6ae4 100644 --- a/storage/innobase/trx/trx0purge.cc +++ b/storage/innobase/trx/trx0purge.cc @@ -1189,7 +1189,10 @@ dict_table_t *purge_sys_t::close_and_reopen(table_id_t id, THD *thd, dict_table_t *table= trx_purge_table_open(id, mdl_context, mdl); if (table == reinterpret_cast(-1)) + { + VALGRIND_YIELD; goto retry; + } for (que_thr_t *thr= UT_LIST_GET_FIRST(purge_sys.query->thrs); thr; thr= UT_LIST_GET_NEXT(thrs, thr)) @@ -1205,6 +1208,7 @@ dict_table_t *purge_sys_t::close_and_reopen(table_id_t id, THD *thd, { if (table) dict_table_close(table, thd, *mdl); + VALGRIND_YIELD; goto retry; } }