Apply AI agent review fixes to PR #5374

Critical Fixes:
- Fix NULL pointer dereferences in MySQL_Logger.cpp:
  * Add null checks for myds->myconn->parent in log_audit_entry()
  * Fix cleanup code in log_request() and log_audit_entry()
  * Change sprintf to snprintf for buffer safety

- Remove redundant pthread_rwlock_init in debug.cpp:
  * filters_rwlock already initialized with PTHREAD_RWLOCK_INITIALIZER
  * Re-initialization caused undefined behavior per POSIX

Major Improvements:
- Add bounded wait helper wait_for_glo_mth() in proxysql_utils.h:
  * Replaces indefinite while loops with 10s timeout
  * Applied to 26 locations across 4 files

Build System:
- Require C++17 in Makefile (remove C++11 fallback):
  * Code uses C++17 structured bindings extensively
  * Cleanest solution vs adding fallback guards to 27+ locations

Compilation fixes:
- Add unistd.h include for usleep
- Remove conflicting GloMyMon extern declaration

Files changed:
- Makefile
- include/proxysql_utils.h
- lib/MySQL_Logger.cpp
- lib/debug.cpp
- lib/MySQL_Monitor.cpp
- lib/ProxySQL_Admin.cpp
- lib/ClickHouse_Server.cpp
- src/SQLite3_Server.cpp
v3.0-test0213
Rene Cannao 5 days ago
parent 941e6b7320
commit 200fdecb8d

@ -74,16 +74,12 @@ DEBUG := $(ALL_DEBUG)
#export EXTRALINK
export MAKE
### detect compiler support for c++11/17
### detect compiler support for c++17 (required)
CPLUSPLUS := $(shell ${CC} -std=c++17 -dM -E -x c++ /dev/null 2>/dev/null | grep -F __cplusplus | egrep -o '[0-9]{6}L')
ifneq ($(CPLUSPLUS),201703L)
CPLUSPLUS := $(shell ${CC} -std=c++11 -dM -E -x c++ /dev/null 2>/dev/null| grep -F __cplusplus | egrep -o '[0-9]{6}L')
LEGACY_BUILD := 1
ifneq ($(CPLUSPLUS),201103L)
$(error Compiler must support at least c++11)
endif
$(error Compiler must support at least c++17)
endif
STDCPP := -std=c++$(shell echo $(CPLUSPLUS) | cut -c3-4) -DCXX$(shell echo $(CPLUSPLUS) | cut -c3-4)
STDCPP := -std=c++17 -DCXX17
### detect distro
DISTRO := Unknown

@ -11,6 +11,7 @@
#include <sys/types.h>
#include <dirent.h>
#include <sys/resource.h>
#include <unistd.h>
#include <assert.h>
#include "../deps/json/json.hpp"
@ -371,4 +372,24 @@ std::string get_client_addr(struct sockaddr* client_addr);
*/
int check_port_availability(int port_num, bool* port_free);
// Forward declaration for GloMTH wait helper
class MySQL_Threads_Handler;
extern MySQL_Threads_Handler *GloMTH;
/**
* @brief Wait for GloMTH to be initialized with a bounded timeout
*
* Waits up to 10 seconds for GloMTH initialization.
* Returns true if GloMTH is initialized, false if timeout.
*
* @return bool true if GloMTH is ready, false otherwise
*/
static inline bool wait_for_glo_mth() {
for (int i = 0; i < 200; ++i) { // ~10s total
if (GloMTH) return true;
usleep(50000);
}
return false;
}
#endif

@ -11,6 +11,7 @@
#include "MySQL_Logger.hpp"
#include "MySQL_Data_Stream.h"
#include "MySQL_Query_Processor.h"
#include "proxysql_utils.h"
#include <search.h>
#include <stdlib.h>
@ -1387,11 +1388,8 @@ static void *child_mysql(void *arg) {
nfds_t nfds=1;
int rc;
pthread_mutex_unlock(&sock_mutex);
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
if (!GloMTH) return NULL;
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
MySQL_Thread *mysql_thr=new MySQL_Thread();
mysql_thr->curtime=monotonic_time();

@ -1519,7 +1519,7 @@ void MySQL_Logger::log_request(MySQL_Session *sess, MySQL_Data_Stream *myds, con
sl+=strlen(sa);
if (sl && myds && myds->myconn && myds->myconn->parent && myds->myconn->parent->port) {
sa=(char *)malloc(sl+9);
sprintf(sa,"%s:%d", myds->myconn->parent->address, myds->myconn->parent->port);
snprintf(sa, sl+9, "%s:%d", myds->myconn->parent->address, myds->myconn->parent->port);
}
sl=strlen(sa);
if (sl) {
@ -1562,7 +1562,7 @@ void MySQL_Logger::log_request(MySQL_Session *sess, MySQL_Data_Stream *myds, con
if (cl && sess->client_myds->addr.port) {
free(ca);
}
if (sl && myds->myconn->parent->port) {
if (sl && myds && myds->myconn && myds->myconn->parent && myds->myconn->parent->port) {
free(sa);
}
}
@ -1676,15 +1676,13 @@ void MySQL_Logger::log_audit_entry(log_event_type _et, MySQL_Session *sess, MySQ
*/
int sl=0;
char *sa=(char *)""; // default
if (myds) {
if (myds->myconn) {
sa=myds->myconn->parent->address;
}
if (myds && myds->myconn && myds->myconn->parent) {
sa=myds->myconn->parent->address;
}
sl+=strlen(sa);
if (sl && myds->myconn->parent->port) {
if (sl && myds && myds->myconn && myds->myconn->parent && myds->myconn->parent->port) {
sa=(char *)malloc(sl+9);
sprintf(sa,"%s:%d", myds->myconn->parent->address, myds->myconn->parent->port);
snprintf(sa, sl+9, "%s:%d", myds->myconn->parent->address, myds->myconn->parent->port);
}
sl=strlen(sa);
@ -1710,7 +1708,7 @@ void MySQL_Logger::log_audit_entry(log_event_type _et, MySQL_Session *sess, MySQ
if (cl && sess->client_myds->addr.port) {
free(ca);
}
if (sl && myds->myconn->parent->port) {
if (sl && myds && myds->myconn && myds->myconn->parent && myds->myconn->parent->port) {
free(sa);
}
}

@ -778,10 +778,8 @@ void * monitor_connect_pthread(void *arg) {
mallctl("thread.tcache.enabled", NULL, NULL, &cache, sizeof(bool));
#endif
set_thread_name("MonitorConnect", GloVars.set_thread_name);
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
GloMyMon->monitor_connect();
return NULL;
}
@ -792,10 +790,8 @@ void * monitor_ping_pthread(void *arg) {
mallctl("thread.tcache.enabled", NULL, NULL, &cache, sizeof(bool));
#endif
set_thread_name("MonitorPing", GloVars.set_thread_name);
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
GloMyMon->monitor_ping();
return NULL;
}
@ -806,10 +802,8 @@ void * monitor_read_only_pthread(void *arg) {
mallctl("thread.tcache.enabled", NULL, NULL, &cache, sizeof(bool));
#endif
set_thread_name("MonitorReadOnly", GloVars.set_thread_name);
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
GloMyMon->monitor_read_only();
return NULL;
}
@ -820,10 +814,8 @@ void * monitor_group_replication_pthread(void *arg) {
mallctl("thread.tcache.enabled", NULL, NULL, &cache, sizeof(bool));
#endif
set_thread_name("MonitorGR", GloVars.set_thread_name);
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
// GloMyMon->monitor_group_replication();
GloMyMon->monitor_group_replication_2();
return NULL;
@ -835,10 +827,8 @@ void * monitor_galera_pthread(void *arg) {
mallctl("thread.tcache.enabled", NULL, NULL, &cache, sizeof(bool));
#endif
set_thread_name("MonitorGalera", GloVars.set_thread_name);
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
GloMyMon->monitor_galera();
return NULL;
}
@ -849,10 +839,8 @@ void * monitor_aws_aurora_pthread(void *arg) {
// mallctl("thread.tcache.enabled", NULL, NULL, &cache, sizeof(bool));
//#endif
set_thread_name("MonitorAurora", GloVars.set_thread_name);
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
GloMyMon->monitor_aws_aurora();
return NULL;
}
@ -863,10 +851,8 @@ void * monitor_replication_lag_pthread(void *arg) {
mallctl("thread.tcache.enabled", NULL, NULL, &cache, sizeof(bool));
#endif
set_thread_name("MonitReplicLag", GloVars.set_thread_name);
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
GloMyMon->monitor_replication_lag();
return NULL;
}
@ -1340,11 +1326,8 @@ void * monitor_connect_thread(const std::vector<MySQL_Monitor_State_Data*>& mmsd
mysql_close(mysql_init(NULL));
MySQL_Monitor_State_Data *mmsd = mmsds.front();
// Wait for GloMTH to be initialized
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
if (!GloMTH) return NULL; // quick exit during shutdown/restart
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
MySQL_Thread * mysql_thr = new MySQL_Thread();
mysql_thr->curtime=monotonic_time();
mysql_thr->refresh_variables();
@ -1403,11 +1386,8 @@ void * monitor_ping_thread(const std::vector<MySQL_Monitor_State_Data*>& mmsds)
assert(!mmsds.empty());
mysql_close(mysql_init(NULL));
MySQL_Monitor_State_Data *mmsd = mmsds.front();
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
if (!GloMTH) return NULL; // quick exit during shutdown/restart
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
MySQL_Thread * mysql_thr = new MySQL_Thread();
mysql_thr->curtime=monotonic_time();
mysql_thr->refresh_variables();
@ -1640,11 +1620,8 @@ void * monitor_read_only_thread(const std::vector<MySQL_Monitor_State_Data*>& da
mysql_close(mysql_init(NULL));
bool timeout_reached = false;
MySQL_Monitor_State_Data *mmsd = data.front();
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
if (!GloMTH) return NULL; // quick exit during shutdown/restart
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
MySQL_Thread * mysql_thr = new MySQL_Thread();
mysql_thr->curtime=monotonic_time();
mysql_thr->refresh_variables();
@ -1922,11 +1899,8 @@ void * monitor_group_replication_thread(const std::vector<MySQL_Monitor_State_Da
assert(!data.empty());
mysql_close(mysql_init(NULL));
MySQL_Monitor_State_Data *mmsd = data.front();
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
if (!GloMTH) return NULL; // quick exit during shutdown/restart
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
MySQL_Thread * mysql_thr = new MySQL_Thread();
mysql_thr->curtime=monotonic_time();
mysql_thr->refresh_variables();
@ -2278,11 +2252,8 @@ void * monitor_galera_thread(const std::vector<MySQL_Monitor_State_Data*>& data)
assert(!data.empty());
mysql_close(mysql_init(NULL));
MySQL_Monitor_State_Data *mmsd = data.front();
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
if (!GloMTH) return NULL; // quick exit during shutdown/restart
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
MySQL_Thread * mysql_thr = new MySQL_Thread();
mysql_thr->curtime=monotonic_time();
mysql_thr->refresh_variables();
@ -2683,11 +2654,8 @@ void * monitor_replication_lag_thread(const std::vector<MySQL_Monitor_State_Data
assert(!data.empty());
mysql_close(mysql_init(NULL));
MySQL_Monitor_State_Data *mmsd = data.front();
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
if (!GloMTH) return NULL; // quick exit during shutdown/restart
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
MySQL_Thread * mysql_thr = new MySQL_Thread();
mysql_thr->curtime=monotonic_time();
mysql_thr->refresh_variables();
@ -3048,11 +3016,8 @@ __fast_exit_monitor_replication_lag_thread:
void * MySQL_Monitor::monitor_connect() {
// initialize the MySQL Thread (note: this is not a real thread, just the structures associated with it)
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
if (!GloMTH) return NULL; // quick exit during shutdown/restart
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
unsigned int MySQL_Monitor__thread_MySQL_Thread_Variables_version;
MySQL_Thread * mysql_thr = new MySQL_Thread();
mysql_thr->curtime=monotonic_time();
@ -3177,11 +3142,8 @@ __sleep_monitor_connect_loop:
void * MySQL_Monitor::monitor_ping() {
mysql_close(mysql_init(NULL));
// initialize the MySQL Thread (note: this is not a real thread, just the structures associated with it)
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
if (!GloMTH) return NULL; // quick exit during shutdown/restart
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
// struct event_base *libevent_base;
unsigned int MySQL_Monitor__thread_MySQL_Thread_Variables_version;
MySQL_Thread * mysql_thr = new MySQL_Thread();
@ -3529,11 +3491,8 @@ bool MySQL_Monitor::is_aws_rds_multi_az_db_cluster_topology(const std::vector<MY
void * MySQL_Monitor::monitor_read_only() {
mysql_close(mysql_init(NULL));
// initialize the MySQL Thread (note: this is not a real thread, just the structures associated with it)
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
if (!GloMTH) return NULL; // quick exit during shutdown/restart
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
unsigned int MySQL_Monitor__thread_MySQL_Thread_Variables_version;
MySQL_Thread * mysql_thr = new MySQL_Thread();
mysql_thr->curtime=monotonic_time();
@ -4157,10 +4116,8 @@ void async_gr_mon_actions_handler(MySQL_Monitor_State_Data* mmsd) {
* @return The created and initialized 'MySQL_Thread'.
*/
unique_ptr<MySQL_Thread> init_mysql_thread_struct() {
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
if (!GloMTH) return NULL;
unique_ptr<MySQL_Thread> mysql_thr { new MySQL_Thread() };
mysql_thr->curtime = monotonic_time();
@ -4407,11 +4364,8 @@ void* MySQL_Monitor::monitor_group_replication_2() {
void * MySQL_Monitor::monitor_group_replication() {
mysql_close(mysql_init(NULL));
// initialize the MySQL Thread (note: this is not a real thread, just the structures associated with it)
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
if (!GloMTH) return NULL; // quick exit during shutdown/restart
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
// struct event_base *libevent_base;
unsigned int MySQL_Monitor__thread_MySQL_Thread_Variables_version;
MySQL_Thread * mysql_thr = new MySQL_Thread();
@ -4518,11 +4472,8 @@ __sleep_monitor_group_replication:
void * MySQL_Monitor::monitor_galera() {
mysql_close(mysql_init(NULL));
// initialize the MySQL Thread (note: this is not a real thread, just the structures associated with it)
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
if (!GloMTH) return NULL; // quick exit during shutdown/restart
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
// struct event_base *libevent_base;
unsigned int MySQL_Monitor__thread_MySQL_Thread_Variables_version;
MySQL_Thread * mysql_thr = new MySQL_Thread();
@ -4593,11 +4544,8 @@ __sleep_monitor_galera:
void * MySQL_Monitor::monitor_replication_lag() {
mysql_close(mysql_init(NULL));
// initialize the MySQL Thread (note: this is not a real thread, just the structures associated with it)
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
if (!GloMTH) return NULL; // quick exit during shutdown/restart
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
unsigned int MySQL_Monitor__thread_MySQL_Thread_Variables_version;
MySQL_Thread * mysql_thr = new MySQL_Thread();
mysql_thr->curtime=monotonic_time();
@ -5167,10 +5115,8 @@ void* MySQL_Monitor::monitor_dns_cache() {
void * MySQL_Monitor::run() {
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
// initialize the MySQL Thread (note: this is not a real thread, just the structures associated with it)
unsigned int MySQL_Monitor__thread_MySQL_Thread_Variables_version;
MySQL_Thread * mysql_thr = new MySQL_Thread();
@ -6062,11 +6008,8 @@ void * monitor_AWS_Aurora_thread_HG(void *arg) {
set_thread_name("MonitorAuroraHG", GloVars.set_thread_name);
proxy_info("Started Monitor thread for AWS Aurora writer HG %u\n", wHG);
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
if (!GloMTH) return NULL; // quick exit during shutdown/restart
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
unsigned int MySQL_Monitor__thread_MySQL_Thread_Variables_version;
MySQL_Thread * mysql_thr = new MySQL_Thread();
mysql_thr->curtime=monotonic_time();
@ -6520,11 +6463,8 @@ __exit_monitor_AWS_Aurora_thread_HG_now:
void * MySQL_Monitor::monitor_aws_aurora() {
// initialize the MySQL Thread (note: this is not a real thread, just the structures associated with it)
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
if (!GloMTH) return NULL; // quick exit during shutdown/restart
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
unsigned int MySQL_Monitor__thread_MySQL_Thread_Variables_version;
MySQL_Thread * mysql_thr = new MySQL_Thread();
mysql_thr->curtime=monotonic_time();

@ -2099,10 +2099,8 @@ void *child_mysql(void *arg) {
nfds_t nfds=1;
int rc;
pthread_mutex_unlock(&sock_mutex);
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
if (!GloMTH) return NULL;
MySQL_Thread *mysql_thr=new MySQL_Thread();
mysql_thr->curtime=monotonic_time();

@ -524,10 +524,9 @@ void print_backtrace(void)
}
#ifdef DEBUG
void init_debug_struct() {
void init_debug_struct() {
int i;
pthread_mutex_init(&debug_mutex,NULL);
pthread_rwlock_init(&filters_rwlock, NULL);
GloVars.global.gdbg_lvl= (debug_level *) malloc(PROXY_DEBUG_UNKNOWN*sizeof(debug_level));
for (i=0;i<PROXY_DEBUG_UNKNOWN;i++) {
GloVars.global.gdbg_lvl[i].module=(enum debug_module)i;

@ -1049,10 +1049,8 @@ static void *child_mysql(void *arg) {
nfds_t nfds=1;
int rc;
pthread_mutex_unlock(&sock_mutex);
while (GloMTH==NULL) {
usleep(50000);
}
usleep(100000);
// Wait for GloMTH to be initialized
if (!wait_for_glo_mth()) return NULL; // quick exit during shutdown/restart
if (!GloMTH) return NULL;
MySQL_Thread *mysql_thr=new MySQL_Thread();
mysql_thr->curtime=monotonic_time();

Loading…
Cancel
Save