From cf7da4e9a238a53523844cd514f2f759512e9822 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jaramago=20Fern=C3=A1ndez?= Date: Tue, 14 May 2024 09:01:54 +0200 Subject: [PATCH] Fix memory leak in 'pull_mysql_query_rules_from_peer' - Simplified memory managament in Cluster credentials API. - Moved several variables into local scopes. --- include/ProxySQL_Cluster.hpp | 7 +- lib/ProxySQL_Cluster.cpp | 177 +++++++++++++++-------------------- 2 files changed, 81 insertions(+), 103 deletions(-) diff --git a/include/ProxySQL_Cluster.hpp b/include/ProxySQL_Cluster.hpp index eab5292cc..92d81efde 100644 --- a/include/ProxySQL_Cluster.hpp +++ b/include/ProxySQL_Cluster.hpp @@ -378,6 +378,11 @@ struct fetch_query { std::string msgs[3]; }; +struct cluster_creds_t { + string user; + string pass; +}; + class ProxySQL_Cluster { private: SQLite3DB* mydb; @@ -444,7 +449,7 @@ public: MySQL_Monitor::trigger_dns_cache_update(); } - void get_credentials(char**, char**); + cluster_creds_t get_credentials(); void set_username(char*); void set_password(char*); void set_admin_mysql_ifaces(char*); diff --git a/lib/ProxySQL_Cluster.cpp b/lib/ProxySQL_Cluster.cpp index 514182b49..f805ed9da 100644 --- a/lib/ProxySQL_Cluster.cpp +++ b/lib/ProxySQL_Cluster.cpp @@ -63,8 +63,6 @@ void * ProxySQL_Cluster_Monitor_thread(void *args) { char *query1 = (char *)"SELECT GLOBAL_CHECKSUM()"; // in future this will be used for "light check" char *query2 = (char *)"SELECT * FROM stats_mysql_global ORDER BY Variable_Name"; char *query3 = (char *)"SELECT * FROM runtime_checksums_values ORDER BY name"; - char *username = NULL; - char *password = NULL; bool rc_bool = true; int query_error_counter = 0; char *query_error = NULL; @@ -76,13 +74,9 @@ void * ProxySQL_Cluster_Monitor_thread(void *args) { goto __exit_monitor_thread; } while (glovars.shutdown == 0 && rc_bool == true) { - MYSQL * rc_conn = NULL; - int rc_query = 0; - bool update_checksum = false; - if (username) { free(username); } - if (password) { free(password); } - GloProxyCluster->get_credentials(&username, &password); - if (strlen(username)) { // do not monitor if the username is empty + cluster_creds_t creds { GloProxyCluster->get_credentials() }; + + if (creds.user.size()) { // do not monitor if the username is empty unsigned int timeout = 1; // unsigned int timeout_long = 60; if (conn == NULL) { @@ -101,14 +95,14 @@ void * ProxySQL_Cluster_Monitor_thread(void *args) { } //rc_conn = mysql_real_connect(conn, node->hostname, username, password, NULL, node->port, NULL, CLIENT_COMPRESS); // FIXME: add optional support for compression proxy_debug(PROXY_DEBUG_CLUSTER, 5, "Connecting to peer %s:%d\n", node->hostname, node->port); - rc_conn = mysql_real_connect(conn, node->get_host_address(), username, password, NULL, node->port, NULL, 0); + MYSQL* rc_conn = mysql_real_connect(conn, node->get_host_address(), creds.user.c_str(), creds.pass.c_str(), NULL, node->port, NULL, 0); // if (rc_conn) { // } //char *query = query1; if (rc_conn) { MySQL_Monitor::update_dns_cache_from_mysql_conn(conn); - rc_query = mysql_query(conn,(char *)"SELECT @@version"); + int rc_query = mysql_query(conn,(char *)"SELECT @@version"); if (rc_query == 0) { query_error = NULL; query_error_counter = 0; @@ -159,7 +153,7 @@ void * ProxySQL_Cluster_Monitor_thread(void *args) { MYSQL_RES *result = mysql_store_result(conn); //unsigned long long after_query_time=monotonic_time(); //unsigned long long elapsed_time_us = (after_query_time - before_query_time); - update_checksum = GloProxyCluster->Update_Global_Checksum(node->hostname, node->port, result); + bool update_checksum = GloProxyCluster->Update_Global_Checksum(node->hostname, node->port, result); mysql_free_result(result); // FIXME: update metrics are not updated for now. We only check checksum //rc_bool = GloProxyCluster->Update_Node_Metrics(node->hostname, node->port, result, elapsed_time_us); @@ -202,7 +196,10 @@ void * ProxySQL_Cluster_Monitor_thread(void *args) { if (query_error_counter == 0) { unsigned long long after_query_time=monotonic_time(); unsigned long long elapsed_time_us = (after_query_time - before_query_time); - proxy_error("Cluster: unable to run query on %s:%d using user %s after %llums : %s . Error: %s\n", node->hostname, node->port , username, elapsed_time_us/1000 , query_error, mysql_error(conn)); + proxy_error( + "Cluster: unable to run query on %s:%d using user %s after %llums : %s . Error: %s\n", + node->hostname, node->port, creds.user.c_str(), elapsed_time_us/1000, query_error, mysql_error(conn) + ); } if (++query_error_counter == QUERY_ERROR_RATE) query_error_counter = 0; } @@ -235,7 +232,10 @@ void * ProxySQL_Cluster_Monitor_thread(void *args) { if (query_error_counter == 0) { unsigned long long after_query_time=monotonic_time(); unsigned long long elapsed_time_us = (after_query_time - before_query_time); - proxy_error("Cluster: unable to run query on %s:%d using user %s after %llums : %s . Error: %s\n", node->hostname, node->port , username, elapsed_time_us/1000 , query_error, mysql_error(conn)); + proxy_error( + "Cluster: unable to run query on %s:%d using user %s after %llums : %s . Error: %s\n", + node->hostname, node->port, creds.user.c_str(), elapsed_time_us/1000, query_error, mysql_error(conn) + ); } if (++query_error_counter == QUERY_ERROR_RATE) query_error_counter = 0; } @@ -246,7 +246,10 @@ void * ProxySQL_Cluster_Monitor_thread(void *args) { if (query_error_counter == 0) { unsigned long long after_query_time=monotonic_time(); unsigned long long elapsed_time_us = (after_query_time - start_time); - proxy_error("Cluster: unable to run query on %s:%d using user %s after %llums : %s . Error: %s\n", node->hostname, node->port , username, elapsed_time_us/1000, query_error, mysql_error(conn)); + proxy_error( + "Cluster: unable to run query on %s:%d using user %s after %llums : %s . Error: %s\n", + node->hostname, node->port, creds.user.c_str(), elapsed_time_us/1000, query_error, mysql_error(conn) + ); } if (++query_error_counter == QUERY_ERROR_RATE) query_error_counter = 0; } @@ -1176,19 +1179,16 @@ void ProxySQL_Cluster::pull_mysql_query_rules_from_peer(const string& expected_c pthread_mutex_lock(&GloProxyCluster->update_mysql_query_rules_mutex); nodes.get_peer_to_sync_mysql_query_rules(&hostname, &port, &ip_address); if (hostname) { - char *username = NULL; - char *password = NULL; - // bool rc_bool = true; - MYSQL *rc_conn; - int rc_query; - int rc; + cluster_creds_t creds {}; + MYSQL *conn = mysql_init(NULL); if (conn==NULL) { proxy_error("Unable to run mysql_init()\n"); goto __exit_pull_mysql_query_rules_from_peer; } - GloProxyCluster->get_credentials(&username, &password); - if (strlen(username)) { // do not monitor if the username is empty + + creds = GloProxyCluster->get_credentials(); + if (creds.user.size()) { // do not monitor if the username is empty unsigned int timeout = 1; // unsigned int timeout_long = 60; mysql_options(conn, MYSQL_OPT_CONNECT_TIMEOUT, &timeout); @@ -1200,16 +1200,18 @@ void ProxySQL_Cluster::pull_mysql_query_rules_from_peer(const string& expected_c } proxy_debug(PROXY_DEBUG_CLUSTER, 5, "Fetching MySQL Query Rules from peer %s:%d started. Expected checksum: %s\n", hostname, port, expected_checksum.c_str()); proxy_info("Cluster: Fetching MySQL Query Rules from peer %s:%d started. Expected checksum: %s\n", hostname, port, expected_checksum.c_str()); - rc_conn = mysql_real_connect(conn, ip_address ? ip_address : hostname, username, password, NULL, port, NULL, 0); + MYSQL* rc_conn = mysql_real_connect( + conn, ip_address ? ip_address : hostname, creds.user.c_str(), creds.pass.c_str(), NULL, port, NULL, 0 + ); if (rc_conn) { MySQL_Monitor::update_dns_cache_from_mysql_conn(conn); MYSQL_RES *result1 = NULL; MYSQL_RES *result2 = NULL; //rc_query = mysql_query(conn,"SELECT rule_id, username, schemaname, flagIN, client_addr, proxy_addr, proxy_port, digest, match_digest, match_pattern, negate_match_pattern, re_modifiers, flagOUT, replace_pattern, destination_hostgroup, cache_ttl, cache_empty_result, cache_timeout, reconnect, timeout, retries, delay, next_query_flagIN, mirror_flagOUT, mirror_hostgroup, error_msg, ok_msg, sticky_conn, multiplex, gtid_from_hostgroup, log, apply, attributes, comment FROM runtime_mysql_query_rules"); - rc_query = mysql_query(conn,CLUSTER_QUERY_MYSQL_QUERY_RULES); + int rc_query = mysql_query(conn,CLUSTER_QUERY_MYSQL_QUERY_RULES); if ( rc_query == 0 ) { - MYSQL_RES *result1 = mysql_store_result(conn); + result1 = mysql_store_result(conn); rc_query = mysql_query(conn,CLUSTER_QUERY_MYSQL_QUERY_RULES_FAST_ROUTING); if ( rc_query == 0) { result2 = mysql_store_result(conn); @@ -1237,7 +1239,7 @@ void ProxySQL_Cluster::pull_mysql_query_rules_from_peer(const string& expected_c sqlite3_stmt *statement1 = NULL; //sqlite3 *mydb3 = GloAdmin->admindb->get_db(); //rc=(*proxy_sqlite3_prepare_v2)(mydb3, q, -1, &statement1, 0); - rc = GloAdmin->admindb->prepare_v2(q, &statement1); + int rc = GloAdmin->admindb->prepare_v2(q, &statement1); ASSERT_SQLITE_OK(rc, GloAdmin->admindb); GloAdmin->admindb->execute("BEGIN TRANSACTION"); while ((row = mysql_fetch_row(result1))) { @@ -1470,17 +1472,16 @@ void ProxySQL_Cluster::pull_mysql_users_from_peer(const string& expected_checksu pthread_mutex_lock(&GloProxyCluster->update_mysql_users_mutex); nodes.get_peer_to_sync_mysql_users(&hostname, &port, &ip_address); if (hostname) { - char *username = NULL; - char *password = NULL; - MYSQL *rc_conn; - int rc_query; + cluster_creds_t creds {}; + MYSQL *conn = mysql_init(NULL); if (conn==NULL) { proxy_error("Unable to run mysql_init()\n"); goto __exit_pull_mysql_users_from_peer; } - GloProxyCluster->get_credentials(&username, &password); - if (strlen(username)) { // do not monitor if the username is empty + + creds = GloProxyCluster->get_credentials(); + if (creds.user.size()) { // do not monitor if the username is empty unsigned int timeout = 1; // unsigned int timeout_long = 60; mysql_options(conn, MYSQL_OPT_CONNECT_TIMEOUT, &timeout); @@ -1493,7 +1494,7 @@ void ProxySQL_Cluster::pull_mysql_users_from_peer(const string& expected_checksu proxy_debug(PROXY_DEBUG_CLUSTER, 5, "Fetching MySQL Users from peer %s:%d started. Expected checksum: %s\n", hostname, port, expected_checksum.c_str()); proxy_info("Cluster: Fetching MySQL Users from peer %s:%d started. Expected checksum: %s\n", hostname, port, expected_checksum.c_str()); - rc_conn = mysql_real_connect(conn, ip_address ? ip_address : hostname, username, password, NULL, port, NULL, 0); + MYSQL* rc_conn = mysql_real_connect(conn, ip_address ? ip_address : hostname, creds.user.c_str(), creds.pass.c_str(), NULL, port, NULL, 0); if (rc_conn == nullptr) { proxy_debug(PROXY_DEBUG_CLUSTER, 5, "Fetching MySQL Users from peer %s:%d failed: %s\n", hostname, port, mysql_error(conn)); proxy_info("Cluster: Fetching MySQL Users from peer %s:%d failed: %s\n", hostname, port, mysql_error(conn)); @@ -1512,7 +1513,7 @@ void ProxySQL_Cluster::pull_mysql_users_from_peer(const string& expected_checksu MySQL_Monitor::update_dns_cache_from_mysql_conn(conn); - rc_query = mysql_query(conn, CLUSTER_QUERY_MYSQL_USERS); + int rc_query = mysql_query(conn, CLUSTER_QUERY_MYSQL_USERS); if (rc_query == 0) { MYSQL_RES* mysql_users_result = mysql_store_result(conn); MYSQL_RES* ldap_mapping_result = nullptr; @@ -1621,12 +1622,6 @@ void ProxySQL_Cluster::pull_mysql_users_from_peer(const string& expected_checksu } } } - if (username) { - free(username); - } - if (password) { - free(password); - } __exit_pull_mysql_users_from_peer: if (conn) { if (conn->net.pvio) { @@ -1771,17 +1766,16 @@ void ProxySQL_Cluster::pull_runtime_mysql_servers_from_peer(const runtime_mysql_ pthread_mutex_lock(&GloProxyCluster->update_runtime_mysql_servers_mutex); nodes.get_peer_to_sync_runtime_mysql_servers(&hostname, &port, &peer_checksum, &ip_address); if (hostname) { - char *username = NULL; - char *password = NULL; - // bool rc_bool = true; - MYSQL *rc_conn; + cluster_creds_t creds {}; + MYSQL *conn = mysql_init(NULL); if (conn==NULL) { proxy_error("Unable to run mysql_init()\n"); goto __exit_pull_mysql_servers_from_peer; } - GloProxyCluster->get_credentials(&username, &password); - if (strlen(username)) { // do not monitor if the username is empty + + creds = GloProxyCluster->get_credentials(); + if (creds.user.size()) { // do not monitor if the username is empty unsigned int timeout = 1; mysql_options(conn, MYSQL_OPT_CONNECT_TIMEOUT, &timeout); { @@ -1790,7 +1784,9 @@ void ProxySQL_Cluster::pull_runtime_mysql_servers_from_peer(const runtime_mysql_ } proxy_debug(PROXY_DEBUG_CLUSTER, 5, "Fetching 'MySQL Servers' from peer %s:%d started. Expected checksum %s\n", hostname, port, peer_checksum); proxy_info("Cluster: Fetching 'MySQL Servers' from peer %s:%d started. Expected checksum %s\n", hostname, port, peer_checksum); - rc_conn = mysql_real_connect(conn, ip_address ? ip_address : hostname, username, password, NULL, port, NULL, 0); + MYSQL* rc_conn = mysql_real_connect( + conn, ip_address ? ip_address : hostname, creds.user.c_str(), creds.pass.c_str(), NULL, port, NULL, 0 + ); if (rc_conn) { MySQL_Monitor::update_dns_cache_from_mysql_conn(conn); @@ -1856,12 +1852,6 @@ void ProxySQL_Cluster::pull_runtime_mysql_servers_from_peer(const runtime_mysql_ fetch_failed = true; } } - if (username) { - free(username); - } - if (password) { - free(password); - } __exit_pull_mysql_servers_from_peer: if (conn) { if (conn->net.pvio) { @@ -1924,17 +1914,16 @@ void ProxySQL_Cluster::pull_mysql_servers_v2_from_peer(const mysql_servers_v2_ch nodes.get_peer_to_sync_mysql_servers_v2(&hostname, &port, &peer_mysql_servers_v2_checksum, &peer_runtime_mysql_servers_checksum, &ip_address); if (hostname) { - char* username = NULL; - char* password = NULL; - // bool rc_bool = true; - MYSQL* rc_conn; + cluster_creds_t creds {}; + MYSQL* conn = mysql_init(NULL); if (conn == NULL) { proxy_error("Unable to run mysql_init()\n"); goto __exit_pull_mysql_servers_v2_from_peer; } - GloProxyCluster->get_credentials(&username, &password); - if (strlen(username)) { // do not monitor if the username is empty + + creds = GloProxyCluster->get_credentials(); + if (creds.user.size()) { // do not monitor if the username is empty unsigned int timeout = 1; // unsigned int timeout_long = 60; mysql_options(conn, MYSQL_OPT_CONNECT_TIMEOUT, &timeout); @@ -1946,7 +1935,9 @@ void ProxySQL_Cluster::pull_mysql_servers_v2_from_peer(const mysql_servers_v2_ch } proxy_debug(PROXY_DEBUG_CLUSTER, 5, "Fetching MySQL Servers v2 from peer %s:%d started. Expected checksum %s\n", hostname, port, peer_mysql_servers_v2_checksum); proxy_info("Cluster: Fetching MySQL Servers v2 from peer %s:%d started. Expected checksum %s\n", hostname, port, peer_mysql_servers_v2_checksum); - rc_conn = mysql_real_connect(conn, ip_address ? ip_address : hostname, username, password, NULL, port, NULL, 0); + MYSQL* rc_conn = mysql_real_connect( + conn, ip_address ? ip_address : hostname, creds.user.c_str(), creds.pass.c_str(), NULL, port, NULL, 0 + ); if (rc_conn) { MySQL_Monitor::update_dns_cache_from_mysql_conn(conn); @@ -2395,12 +2386,6 @@ void ProxySQL_Cluster::pull_mysql_servers_v2_from_peer(const mysql_servers_v2_ch fetch_failed = true; } } - if (username) { - free(username); - } - if (password) { - free(password); - } __exit_pull_mysql_servers_v2_from_peer: if (conn) { if (conn->net.pvio) { @@ -2461,20 +2446,16 @@ void ProxySQL_Cluster::pull_global_variables_from_peer(const string& var_type, c } if (hostname) { - char *username = NULL; - char *password = NULL; - MYSQL *rc_conn = nullptr; - int rc_query = 0; - int rc = 0; - MYSQL *conn = mysql_init(NULL); + cluster_creds_t creds {}; + MYSQL *conn = mysql_init(NULL); if (conn == NULL) { proxy_error("Unable to run mysql_init()\n"); goto __exit_pull_mysql_variables_from_peer; } - GloProxyCluster->get_credentials(&username, &password); - if (strlen(username)) { // do not monitor if the username is empty + creds = GloProxyCluster->get_credentials(); + if (creds.user.size()) { // do not monitor if the username is empty unsigned int timeout = 1; // unsigned int timeout_long = 60; mysql_options(conn, MYSQL_OPT_CONNECT_TIMEOUT, &timeout); @@ -2485,7 +2466,9 @@ void ProxySQL_Cluster::pull_global_variables_from_peer(const string& var_type, c mysql_options(conn, MARIADB_OPT_SSL_KEYLOG_CALLBACK, (void*)proxysql_keylog_write_line_callback); } proxy_info("Cluster: Fetching %s variables from peer %s:%d started\n", vars_type_str, hostname, port); - rc_conn = mysql_real_connect(conn, ip_address ? ip_address : hostname, username, password, NULL, port, NULL, 0); + MYSQL* rc_conn = mysql_real_connect( + conn, ip_address ? ip_address : hostname, creds.user.c_str(), creds.pass.c_str(), NULL, port, NULL, 0 + ); if (rc_conn) { MySQL_Monitor::update_dns_cache_from_mysql_conn(conn); @@ -2503,7 +2486,7 @@ void ProxySQL_Cluster::pull_global_variables_from_peer(const string& var_type, c } } s_query += " ORDER BY variable_name"; - mysql_query(conn, s_query.c_str()); + int rc_query = mysql_query(conn, s_query.c_str()); if (rc_query == 0) { MYSQL_RES *result = mysql_store_result(conn); @@ -2535,7 +2518,7 @@ void ProxySQL_Cluster::pull_global_variables_from_peer(const string& var_type, c MYSQL_ROW row; char *q = (char *)"INSERT OR REPLACE INTO global_variables (variable_name, variable_value) VALUES (?1 , ?2)"; sqlite3_stmt *statement1 = NULL; - rc = GloAdmin->admindb->prepare_v2(q, &statement1); + int rc = GloAdmin->admindb->prepare_v2(q, &statement1); ASSERT_SQLITE_OK(rc, GloAdmin->admindb); while ((row = mysql_fetch_row(result))) { @@ -2604,12 +2587,6 @@ void ProxySQL_Cluster::pull_global_variables_from_peer(const string& var_type, c fetch_failed = true; } } - if (username) { - free(username); - } - if (password) { - free(password); - } __exit_pull_mysql_variables_from_peer: if (conn) { if (conn->net.pvio) { @@ -2633,18 +2610,16 @@ void ProxySQL_Cluster::pull_proxysql_servers_from_peer(const std::string& expect pthread_mutex_lock(&GloProxyCluster->update_proxysql_servers_mutex); nodes.get_peer_to_sync_proxysql_servers(&hostname, &port, &ip_address); if (hostname) { - char *username = NULL; - char *password = NULL; - // bool rc_bool = true; - MYSQL *rc_conn; - int rc_query; + cluster_creds_t creds {}; + MYSQL *conn = mysql_init(NULL); if (conn==NULL) { proxy_error("Unable to run mysql_init()\n"); goto __exit_pull_proxysql_servers_from_peer; } - GloProxyCluster->get_credentials(&username, &password); - if (strlen(username)) { // do not monitor if the username is empty + + creds = GloProxyCluster->get_credentials(); + if (creds.user.size()) { // do not monitor if the username is empty unsigned int timeout = 1; // unsigned int timeout_long = 60; mysql_options(conn, MYSQL_OPT_CONNECT_TIMEOUT, &timeout); @@ -2660,11 +2635,13 @@ void ProxySQL_Cluster::pull_proxysql_servers_from_peer(const std::string& expect "Cluster: Fetching ProxySQL Servers from peer %s:%d started. Expected checksum: %s\n", hostname, port, expected_checksum.c_str() ); - rc_conn = mysql_real_connect(conn, ip_address ? ip_address : hostname, username, password, NULL, port, NULL, 0); + MYSQL* rc_conn = mysql_real_connect( + conn, ip_address ? ip_address : hostname, creds.user.c_str(), creds.pass.c_str(), NULL, port, NULL, 0 + ); if (rc_conn) { MySQL_Monitor::update_dns_cache_from_mysql_conn(conn); - rc_query = mysql_query(conn,"SELECT hostname, port, weight, comment FROM runtime_proxysql_servers ORDER BY hostname, port"); + int rc_query = mysql_query(conn,"SELECT hostname, port, weight, comment FROM runtime_proxysql_servers ORDER BY hostname, port"); if ( rc_query == 0 ) { MYSQL_RES* result = mysql_store_result(conn); uint64_t proxy_servers_hash = mysql_raw_checksum(result); @@ -2738,12 +2715,6 @@ void ProxySQL_Cluster::pull_proxysql_servers_from_peer(const std::string& expect fetch_failed = true; } } - if (username) { - free(username); - } - if (password) { - free(password); - } __exit_pull_proxysql_servers_from_peer: if (conn) { if (conn->net.pvio) { @@ -4467,11 +4438,13 @@ void ProxySQL_Cluster::p_update_metrics() { }; // this function returns credentials to the caller, used by monitoring threads -void ProxySQL_Cluster::get_credentials(char **username, char **password) { +cluster_creds_t ProxySQL_Cluster::get_credentials() { pthread_mutex_lock(&mutex); - *username = strdup(cluster_username); - *password = strdup(cluster_password); + const string user { cluster_username }; + const string pass { cluster_password }; pthread_mutex_unlock(&mutex); + + return { user, pass }; } void ProxySQL_Cluster::set_username(char *_username) {