From edf427689acc2199e20b7abe5007e6a2e66bfcf2 Mon Sep 17 00:00:00 2001 From: Valentin Rakush Date: Sat, 9 Nov 2019 14:16:55 +0000 Subject: [PATCH] Fix memory leak in ProxySQL ConfigFile. Avoid using pointers because of valgrind crashes. --- include/configfile.hpp | 18 ++++++++---------- lib/ProxySQL_Admin.cpp | 42 ++++++++++++++---------------------------- lib/configfile.cpp | 34 +++++++++------------------------- src/main.cpp | 4 ++-- 4 files changed, 33 insertions(+), 65 deletions(-) diff --git a/include/configfile.hpp b/include/configfile.hpp index 06cbd0bbf..bcf73493a 100644 --- a/include/configfile.hpp +++ b/include/configfile.hpp @@ -9,18 +9,16 @@ using namespace libconfig; class ProxySQL_ConfigFile { private: //struct stat statbuf; - char *filename; + std::string filename; public: - Config *cfg; - ProxySQL_ConfigFile(); + Config cfg; bool OpenFile(const char *); - void CloseFile(); - bool ReadGlobals(); - bool configVariable(const char *, const char *, int &, int, int, int, int); - bool configVariable(const char *, const char *, int64_t &, int64_t, int64_t, int64_t, int64_t); - bool configVariable(const char *, const char *, bool &, bool); - bool configVariable(const char *, const char *, char **, const char *); - ~ProxySQL_ConfigFile(); + void CloseFile(); + bool ReadGlobals(); + bool configVariable(const char *, const char *, int &, int, int, int, int); + bool configVariable(const char *, const char *, int64_t &, int64_t, int64_t, int64_t, int64_t); + bool configVariable(const char *, const char *, bool &, bool); + bool configVariable(const char *, const char *, char **, const char *); }; diff --git a/lib/ProxySQL_Admin.cpp b/lib/ProxySQL_Admin.cpp index be898f2a9..4248cc692 100644 --- a/lib/ProxySQL_Admin.cpp +++ b/lib/ProxySQL_Admin.cpp @@ -4548,27 +4548,14 @@ bool ProxySQL_Admin::init() { if (GloVars.__cmd_proxysql_reload || GloVars.__cmd_proxysql_initial || admindb_file_exists==false) { // see #617 if (GloVars.configfile_open) { - if (GloVars.confFile->cfg) { - Read_MySQL_Servers_from_configfile(); - Read_Global_Variables_from_configfile("admin"); - Read_Global_Variables_from_configfile("mysql"); - Read_MySQL_Users_from_configfile(); - Read_MySQL_Query_Rules_from_configfile(); - Read_Scheduler_from_configfile(); - Read_ProxySQL_Servers_from_configfile(); - __insert_or_replace_disktable_select_maintable(); - } else { - if (GloVars.confFile->OpenFile(GloVars.config_file)==true) { - Read_MySQL_Servers_from_configfile(); - Read_MySQL_Users_from_configfile(); - Read_MySQL_Query_Rules_from_configfile(); - Read_Global_Variables_from_configfile("admin"); - Read_Global_Variables_from_configfile("mysql"); - Read_Scheduler_from_configfile(); - Read_ProxySQL_Servers_from_configfile(); - __insert_or_replace_disktable_select_maintable(); - } - } + Read_MySQL_Servers_from_configfile(); + Read_MySQL_Users_from_configfile(); + Read_MySQL_Query_Rules_from_configfile(); + Read_Global_Variables_from_configfile("admin"); + Read_Global_Variables_from_configfile("mysql"); + Read_Scheduler_from_configfile(); + Read_ProxySQL_Servers_from_configfile(); + __insert_or_replace_disktable_select_maintable(); } } flush_admin_variables___database_to_runtime(admindb,true); @@ -5187,7 +5174,6 @@ bool ProxySQL_Admin::ProxySQL_Test___Verify_mysql_query_rules_fast_routing(int * unsigned int ProxySQL_Admin::ProxySQL_Test___GenerateRandom_mysql_query_rules_fast_routing(unsigned int cnt) { char *a = "INSERT OR IGNORE INTO mysql_query_rules_fast_routing VALUES (?1, ?2, ?3, ?4, '')"; int rc; - unsigned int i=0; sqlite3_stmt *statement1=NULL; rc=admindb->prepare_v2(a, &statement1); ASSERT_SQLITE_OK(rc, admindb); @@ -9008,7 +8994,7 @@ char * ProxySQL_Admin::load_mysql_query_rules_to_runtime() { } int ProxySQL_Admin::Read_Global_Variables_from_configfile(const char *prefix) { - const Setting& root = GloVars.confFile->cfg->getRoot(); + const Setting& root = GloVars.confFile->cfg.getRoot(); char *groupname=(char *)malloc(strlen(prefix)+strlen((char *)"_variables")+1); sprintf(groupname,"%s%s",prefix,"_variables"); if (root.exists(groupname)==false) { @@ -9049,7 +9035,7 @@ int ProxySQL_Admin::Read_Global_Variables_from_configfile(const char *prefix) { } int ProxySQL_Admin::Read_MySQL_Users_from_configfile() { - const Setting& root = GloVars.confFile->cfg->getRoot(); + const Setting& root = GloVars.confFile->cfg.getRoot(); if (root.exists("mysql_users")==false) return 0; const Setting &mysql_users = root["mysql_users"]; int count = mysql_users.getLength(); @@ -9098,7 +9084,7 @@ int ProxySQL_Admin::Read_MySQL_Users_from_configfile() { } int ProxySQL_Admin::Read_Scheduler_from_configfile() { - const Setting& root = GloVars.confFile->cfg->getRoot(); + const Setting& root = GloVars.confFile->cfg.getRoot(); if (root.exists("scheduler")==false) return 0; const Setting &schedulers = root["scheduler"]; int count = schedulers.getLength(); @@ -9198,7 +9184,7 @@ int ProxySQL_Admin::Read_Scheduler_from_configfile() { } int ProxySQL_Admin::Read_MySQL_Query_Rules_from_configfile() { - const Setting& root = GloVars.confFile->cfg->getRoot(); + const Setting& root = GloVars.confFile->cfg.getRoot(); if (root.exists("mysql_query_rules")==false) return 0; const Setting &mysql_query_rules = root["mysql_query_rules"]; int count = mysql_query_rules.getLength(); @@ -9460,7 +9446,7 @@ int ProxySQL_Admin::Read_MySQL_Query_Rules_from_configfile() { } int ProxySQL_Admin::Read_MySQL_Servers_from_configfile() { - const Setting& root = GloVars.confFile->cfg->getRoot(); + const Setting& root = GloVars.confFile->cfg.getRoot(); int i; int rows=0; admindb->execute("PRAGMA foreign_keys = OFF"); @@ -9678,7 +9664,7 @@ int ProxySQL_Admin::Read_MySQL_Servers_from_configfile() { } int ProxySQL_Admin::Read_ProxySQL_Servers_from_configfile() { - const Setting& root = GloVars.confFile->cfg->getRoot(); + const Setting& root = GloVars.confFile->cfg.getRoot(); int i; int rows=0; admindb->execute("PRAGMA foreign_keys = OFF"); diff --git a/lib/configfile.cpp b/lib/configfile.cpp index 9565896f4..9931e7af4 100644 --- a/lib/configfile.cpp +++ b/lib/configfile.cpp @@ -32,21 +32,13 @@ struct _global_configfile_entry_t { void (*func_post)(global_configfile_entry_t *); // function called after initializing variable }; -ProxySQL_ConfigFile::ProxySQL_ConfigFile() { - filename=NULL; -}; - bool ProxySQL_ConfigFile::OpenFile(const char *__filename) { - cfg = new Config(); - if (__filename) { - filename=strdup(__filename); - } else { - assert(filename); - } - if (FileUtils::isReadable(filename)==false) return false; + assert(__filename); + filename = __filename; + if (FileUtils::isReadable(filename.c_str())==false) return false; try { - cfg->readFile(filename); + cfg.readFile(filename.c_str()); } catch(const FileIOException &fioex) { @@ -67,12 +59,6 @@ bool ProxySQL_ConfigFile::OpenFile(const char *__filename) { }; void ProxySQL_ConfigFile::CloseFile() { -/* FIXME - for now we are commenting out this. - It seems that after upgrade to jemalloc 5.2.0 , valgrind crashes here -*/ -// delete cfg; - cfg=NULL; } bool ProxySQL_ConfigFile::ReadGlobals() { @@ -80,7 +66,7 @@ bool ProxySQL_ConfigFile::ReadGlobals() { }; bool ProxySQL_ConfigFile::configVariable(const char *group, const char *key, int &variable, int defValue, int minValue, int maxValue, int multiplier) { - const Setting& root = cfg->getRoot(); + const Setting& root = cfg.getRoot(); if (root.exists(group)==true) { const Setting& mygroup=root[group]; if (mygroup.isGroup()==true) { @@ -119,7 +105,7 @@ bool ProxySQL_ConfigFile::configVariable(const char *group, const char *key, int } bool ProxySQL_ConfigFile::configVariable(const char *group, const char *key, int64_t &variable, int64_t defValue, int64_t minValue, int64_t maxValue, int64_t multiplier) { - const Setting& root = cfg->getRoot(); + const Setting& root = cfg.getRoot(); if (root.exists(group)==true) { const Setting& mygroup=root[group]; if (mygroup.isGroup()==true) { @@ -158,7 +144,7 @@ bool ProxySQL_ConfigFile::configVariable(const char *group, const char *key, int } bool ProxySQL_ConfigFile::configVariable(const char *group, const char *key, bool & variable, bool defValue) { - const Setting& root = cfg->getRoot(); + const Setting& root = cfg.getRoot(); if (root.exists(group)==true) { const Setting& mygroup=root[group]; if (mygroup.isGroup()==true) { @@ -187,7 +173,7 @@ bool ProxySQL_ConfigFile::configVariable(const char *group, const char *key, boo } bool ProxySQL_ConfigFile::configVariable(const char *group, const char *key, char **variable, const char *defValue) { - const Setting& root = cfg->getRoot(); + const Setting& root = cfg.getRoot(); if (root.exists(group)==true) { const Setting& mygroup=root[group]; if (mygroup.isGroup()==true) { @@ -215,6 +201,4 @@ bool ProxySQL_ConfigFile::configVariable(const char *group, const char *key, cha return true; } -ProxySQL_ConfigFile::~ProxySQL_ConfigFile() { - if (filename) free(filename); -}; + diff --git a/src/main.cpp b/src/main.cpp index 0943955f2..17ff06c1e 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -701,7 +701,7 @@ void ProxySQL_Main_process_global_variables(int argc, const char **argv) { if (GloVars.confFile->OpenFile(GloVars.config_file) == true) { GloVars.configfile_open=true; proxy_info("Using config file %s\n", GloVars.config_file); - const Setting& root = GloVars.confFile->cfg->getRoot(); + const Setting& root = GloVars.confFile->cfg.getRoot(); if (root.exists("restart_on_missing_heartbeats")==true) { // restart_on_missing_heartbeats datadir from config file int restart_on_missing_heartbeats; @@ -748,7 +748,7 @@ void ProxySQL_Main_process_global_variables(int argc, const char **argv) { if (GloVars.__cmd_proxysql_datadir==NULL) { // datadir was not specified , try to read config file if (GloVars.configfile_open==true) { - const Setting& root = GloVars.confFile->cfg->getRoot(); + const Setting& root = GloVars.confFile->cfg.getRoot(); if (root.exists("datadir")==true) { // reading datadir from config file std::string datadir;