From 662bb176b412993a085fe329af559ddc3dc83ec3 Mon Sep 17 00:00:00 2001 From: Yuchen Pei Date: Wed, 13 Mar 2024 13:11:15 +1100 Subject: [PATCH] MDEV-33661 MENT-1591 Keep spider in memory until exit in ASAN builds Same as MDEV-29579. For some reason, libodbc does not clean up properly if unloaded too early with the dlclose() of spider. So we add UNIQUE symbols to spider so the spider does not reload in dlclose(). This change, however, uncovers some hidden problems in the spider codebase, for which we move the initialisation of some spider global variables into the initialisation of spider itself. Spider has some global variables. Their initialisation should be done in the initialisation of spider itself, otherwise, if spider were re-initialised without these symbol being unloaded, the values could be inconsistent and causing issues. One such issue is caused by the variables spider_mon_table_cache_version and spider_mon_table_cache_version_req. They are used for resetting the spider monitoring table cache and have initial values of 0 and 1 respectively. We have that always spider_mon_table_cache_version_req >= spider_mon_table_cache_version, and when the relation is strict, the cache is reset, spider_mon_table_cache_version is brought to be equal to spider_mon_table_cache_version_req, and the cache is searched for matching table_name, db_name and link_idx. If the relation is equal, no reset would happen and the cache would be searched directly. When spider is re-inited without resetting the values of spider_mon_table_cache_version and spider_mon_table_cache_version_req that were set to be equal in the previous cache reset action, the cache was emptied in the previous spider deinit, which would result in HA_ERR_KEY_NOT_FOUND unexpectedly. An alternative way to fix this issue would be to call the spider udf spider_flush_mon_cache_table(), which increments spider_mon_table_cache_version_req thus making sure the inequality is strict. However, there's no reason for spider to initialise these global variables on dlopen(), rather than on spider init, which is cleaner and "purer". To reproduce this issue, simply revert the changes involving the two variables and then run: mtr --no-reorder spider.ha{,_part} --- storage/spider/ha_spider.h | 23 +++++++++++++++++++++++ storage/spider/spd_conn.cc | 4 ++-- storage/spider/spd_db_conn.cc | 2 +- storage/spider/spd_ping_table.cc | 4 ++-- storage/spider/spd_table.cc | 11 +++++++++++ storage/spider/spd_trx.cc | 2 +- 6 files changed, 40 insertions(+), 6 deletions(-) diff --git a/storage/spider/ha_spider.h b/storage/spider/ha_spider.h index f5a597b88a7..3434add965c 100644 --- a/storage/spider/ha_spider.h +++ b/storage/spider/ha_spider.h @@ -1219,3 +1219,26 @@ public: int init_union_table_name_pos_sql(); int set_union_table_name_pos_sql(); }; + + +/* This is a hack for ASAN + * Libraries such as libxml2 and libodbc do not like being unloaded before + * exit and will show as a leak in ASAN with no stack trace (as the plugin + * has been unloaded from memory). + * + * The below is designed to trick the compiler into adding a "UNIQUE" symbol + * which can be seen using: + * readelf -s storage/spider/ha_spider.so | grep UNIQUE + * + * Having this symbol means that the plugin remains in memory after dlclose() + * has been called. Thereby letting the libraries clean up properly. + */ +#if defined(__SANITIZE_ADDRESS__) +__attribute__((__used__)) +inline int dummy(void) +{ + static int d; + d++; + return d; +} +#endif diff --git a/storage/spider/spd_conn.cc b/storage/spider/spd_conn.cc index be9b27d83ad..b55dc0fd4db 100644 --- a/storage/spider/spd_conn.cc +++ b/storage/spider/spd_conn.cc @@ -53,7 +53,7 @@ extern handlerton *spider_hton_ptr; extern SPIDER_DBTON spider_dbton[SPIDER_DBTON_SIZE]; pthread_mutex_t spider_conn_id_mutex; pthread_mutex_t spider_ipport_conn_mutex; -ulonglong spider_conn_id = 1; +ulonglong spider_conn_id; #ifndef WITHOUT_SPIDER_BG_SEARCH extern pthread_attr_t spider_pt_attr; @@ -93,7 +93,7 @@ extern sql_mode_t pushdown_sql_mode; HASH spider_open_connections; uint spider_open_connections_id; HASH spider_ipport_conns; -long spider_conn_mutex_id = 0; +long spider_conn_mutex_id; const char *spider_open_connections_func_name; const char *spider_open_connections_file_name; diff --git a/storage/spider/spd_db_conn.cc b/storage/spider/spd_db_conn.cc index 40dc15a2149..fd10c7cee8b 100644 --- a/storage/spider/spd_db_conn.cc +++ b/storage/spider/spd_db_conn.cc @@ -67,7 +67,7 @@ pthread_mutex_t spider_open_conn_mutex; const char spider_dig_upper[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"; /* UTC time zone for timestamp columns */ -Time_zone *UTC = 0; +Time_zone *UTC; int spider_db_connect( const SPIDER_SHARE *share, diff --git a/storage/spider/spd_ping_table.cc b/storage/spider/spd_ping_table.cc index 4d2c826aa28..1f5230829c3 100644 --- a/storage/spider/spd_ping_table.cc +++ b/storage/spider/spd_ping_table.cc @@ -74,10 +74,10 @@ ulong spider_mon_table_cache_line_no; greater than spider_mon_table_cache_version_req. When the inequality is strict, an initialisation of spider_mon_table_cache will be triggered. */ -volatile ulonglong spider_mon_table_cache_version = 0; +volatile ulonglong spider_mon_table_cache_version; /* The required mon table cache version, incremented by one by the udf spider_flush_table_mon_cache */ -volatile ulonglong spider_mon_table_cache_version_req = 1; +volatile ulonglong spider_mon_table_cache_version_req; /* Get or create a `SPIDER_TABLE_MON_LIST' for a key `str' */ SPIDER_TABLE_MON_LIST *spider_get_ping_table_mon_list( diff --git a/storage/spider/spd_table.cc b/storage/spider/spd_table.cc index 60e1ce6bad2..c1435e0216e 100644 --- a/storage/spider/spd_table.cc +++ b/storage/spider/spd_table.cc @@ -139,6 +139,11 @@ extern SPIDER_DBTON spider_dbton_oracle; SPIDER_THREAD *spider_table_sts_threads; SPIDER_THREAD *spider_table_crd_threads; #endif +extern volatile ulonglong spider_mon_table_cache_version; +extern volatile ulonglong spider_mon_table_cache_version_req; +extern ulonglong spider_conn_id; +extern Time_zone *UTC; +extern ulonglong spider_thread_id; #ifdef HAVE_PSI_INTERFACE PSI_mutex_key spd_key_mutex_tbl; @@ -6614,6 +6619,12 @@ int spider_db_init( handlerton *spider_hton = (handlerton *)p; DBUG_ENTER("spider_db_init"); + spider_mon_table_cache_version= 0; + spider_mon_table_cache_version_req= 1; + spider_conn_id= 1; + spider_conn_mutex_id= 0; + UTC = 0; + spider_thread_id = 1; const LEX_CSTRING aria_name={STRING_WITH_LEN("Aria")}; if (!plugin_is_ready(&aria_name, MYSQL_STORAGE_ENGINE_PLUGIN)) DBUG_RETURN(HA_ERR_RETRY_INIT); diff --git a/storage/spider/spd_trx.cc b/storage/spider/spd_trx.cc index bdea13e7d3b..7a92a6e3f17 100644 --- a/storage/spider/spd_trx.cc +++ b/storage/spider/spd_trx.cc @@ -49,7 +49,7 @@ extern struct charset_info_st *spd_charset_utf8_bin; extern handlerton *spider_hton_ptr; extern SPIDER_DBTON spider_dbton[SPIDER_DBTON_SIZE]; pthread_mutex_t spider_thread_id_mutex; -ulonglong spider_thread_id = 1; +ulonglong spider_thread_id; #ifdef HAVE_PSI_INTERFACE extern PSI_mutex_key spd_key_mutex_udf_table;