Skip to content

Fix memory leak in 'ProxySQL_Admin::stats___mysql_global' - Closes #4976 #5006

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 15, 2025
+71 −17
Merged

Fix memory leak in 'ProxySQL_Admin::stats___mysql_global' - Closes #4976 #5006

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6f2798a
Fix memory leak in 'ProxySQL_Admin::stats___mysql_global' - Closes #4976
JavierJF Jun 25, 2025
25d5681
Fix potential overflow for 'sqlite3_status' memory metrics
JavierJF Jul 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions include/sqlite3db.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
#undef min
#undef max
#include <cstdint>
#include <memory>
#include <vector>
#include <utility>
#define PROXYSQL_SQLITE3DB_PTHREAD_MUTEX

#ifndef SAFE_SQLITE3_STEP2
Expand Down Expand Up @@ -40,6 +42,7 @@ extern int (*proxy_sqlite3_close_v2)(sqlite3*);
extern int (*proxy_sqlite3_get_autocommit)(sqlite3*);
extern void (*proxy_sqlite3_free)(void*);
extern int (*proxy_sqlite3_status)(int op, int *pCurrent, int *pHighwater, int resetFlag);
extern int (*proxy_sqlite3_status64)(int op, long long *pCurrent, long long *pHighwater, int resetFlag);
extern int (*proxy_sqlite3_changes)(sqlite3*);
extern int (*proxy_sqlite3_step)(sqlite3_stmt*);
extern int (*proxy_sqlite3_config)(int, ...);
Expand Down Expand Up @@ -87,6 +90,8 @@ int (*proxy_sqlite3_close_v2)(sqlite3*);
int (*proxy_sqlite3_get_autocommit)(sqlite3*);
void (*proxy_sqlite3_free)(void*);
int (*proxy_sqlite3_status)(int op, int *pCurrent, int *pHighwater, int resetFlag);
int (*proxy_sqlite3_status64)(int op, long long *pCurrent, long long *pHighwater, int resetFlag);

int (*proxy_sqlite3_changes)(sqlite3*);
int (*proxy_sqlite3_step)(sqlite3_stmt*);
int (*proxy_sqlite3_config)(int, ...);
Expand Down Expand Up @@ -165,6 +170,18 @@ class SQLite3_result {
void dump_to_stderr();
};

/**
* @brief Helper type for finalizing 'sqlite3_stmt' managed by smart pointers.
*/
struct stmt_deleter_t {
void operator()(sqlite3_stmt* x) const;
};

/**
* @brief Safe type for automatically deallocation of 'sqlite3_stmt'.
*/
using stmt_unique_ptr = std::unique_ptr<sqlite3_stmt, stmt_deleter_t>;

class SQLite3DB {
private:
char *url;
Expand All @@ -191,7 +208,14 @@ class SQLite3DB {
int check_table_structure(char *table_name, char *table_def);
bool build_table(char *table_name, char *table_def, bool dropit);
bool check_and_build_table(char *table_name, char *table_def);
[[deprecated("Use safer alternative 'prepare_v2(const char *)'")]]
int prepare_v2(const char *, sqlite3_stmt **);
/**
* @brief Prepares a query as a statement in the SQLite3DB.
* @param query The query to be prepared as an 'sqlite3_stmt'.
* @return A pair of with shape { err_code, stmt_unique_ptr }.
*/
std::pair<int,stmt_unique_ptr> prepare_v2(const char* query);
static void LoadPlugin(const char *);
};

Expand Down
41 changes: 24 additions & 17 deletions lib/ProxySQL_Admin_Stats.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,9 @@ void ProxySQL_Admin::p_stats___memory_metrics() {
this->metrics.p_gauge_array[p_admin_gauge::connpool_memory_bytes]->Set(connpool_mem);

// proxysql_sqlite3_memory_bytes metric
int highwater = 0;
int current = 0;
(*proxy_sqlite3_status)(SQLITE_STATUS_MEMORY_USED, &current, &highwater, 0);
long long highwater = 0;
long long current = 0;
(*proxy_sqlite3_status64)(SQLITE_STATUS_MEMORY_USED, &current, &highwater, 0);
this->metrics.p_gauge_array[p_admin_gauge::sqlite3_memory_bytes]->Set(current);

// proxysql_jemalloc_* memory metrics
Expand Down Expand Up @@ -206,8 +206,8 @@ void ProxySQL_Admin::stats___memory_metrics() {
if (!GloMTH) return;
SQLite3_result * resultset = NULL;

int highwater;
int current;
long long highwater = 0;
long long current = 0;
char bu[32];
char *vn=NULL;
char *query=NULL;
Expand All @@ -218,9 +218,9 @@ void ProxySQL_Admin::stats___memory_metrics() {
delete resultset;
resultset=NULL;
}
(*proxy_sqlite3_status)(SQLITE_STATUS_MEMORY_USED, &current, &highwater, 0);
(*proxy_sqlite3_status64)(SQLITE_STATUS_MEMORY_USED, &current, &highwater, 0);
vn=(char *)"SQLite3_memory_bytes";
sprintf(bu,"%d",current);
sprintf(bu,"%lld",current);
query=(char *)malloc(strlen(a)+strlen(vn)+strlen(bu)+16);
sprintf(query,a,vn,bu);
statsdb->execute(query);
Expand Down Expand Up @@ -492,6 +492,8 @@ const void sqlite3_global_stats_row_step(
sprintf(buf, "%lu", val);
} else if constexpr (std::is_same_v<T, unsigned long long>) {
sprintf(buf, "%llu", val);
} else if constexpr (std::is_same_v<T, long long>) {
sprintf(buf, "%lld", val);
} else if constexpr (std::is_same_v<T, bool>) {
sprintf(buf, "%s", val ? "true" : "false");
} else {
Expand Down Expand Up @@ -521,12 +523,17 @@ void ProxySQL_Admin::stats___mysql_global() {
"INSERT INTO stats_mysql_global VALUES " + generate_multi_rows_query(32, 2)
};

sqlite3_stmt* row_stmt = nullptr;
int rc = statsdb->prepare_v2(q_row_insert.c_str(), &row_stmt);
int rc = 0;

stmt_unique_ptr u_row_stmt { nullptr };
std::tie(rc, u_row_stmt) = statsdb->prepare_v2(q_row_insert.c_str());
ASSERT_SQLITE_OK(rc, statsdb);
sqlite3_stmt* bulk_stmt = nullptr;
rc = statsdb->prepare_v2(q_bulk_insert.c_str(), &bulk_stmt);
sqlite3_stmt* const row_stmt { u_row_stmt.get() };

stmt_unique_ptr u_bulk_stmt { nullptr };
std::tie(rc, u_bulk_stmt) = statsdb->prepare_v2(q_bulk_insert.c_str());
ASSERT_SQLITE_OK(rc, statsdb);
sqlite3_stmt* const bulk_stmt { u_bulk_stmt.get() };

sqlite3_bulk_step(statsdb, row_stmt, bulk_stmt, resultset, stats_mysql_global___bind_row);

Expand All @@ -542,8 +549,8 @@ void ProxySQL_Admin::stats___mysql_global() {
}

{
int highwater, current = 0;
(*proxy_sqlite3_status)(SQLITE_STATUS_MEMORY_USED, &current, &highwater, 0);
long long highwater, current = 0;
(*proxy_sqlite3_status64)(SQLITE_STATUS_MEMORY_USED, &current, &highwater, 0);
sqlite3_global_stats_row_step(statsdb, row_stmt, "SQLite3_memory_bytes", current);
}

Expand Down Expand Up @@ -647,14 +654,14 @@ void ProxySQL_Admin::stats___pgsql_global() {
resultset = NULL;
}

int highwater;
int current;
(*proxy_sqlite3_status)(SQLITE_STATUS_MEMORY_USED, &current, &highwater, 0);
long long highwater = 0;
long long current = 0;
(*proxy_sqlite3_status64)(SQLITE_STATUS_MEMORY_USED, &current, &highwater, 0);
char bu[32];
char* vn = NULL;
char* query = NULL;
vn = (char*)"SQLite3_memory_bytes";
sprintf(bu, "%d", current);
sprintf(bu, "%lld", current);
query = (char*)malloc(strlen(a) + strlen(vn) + strlen(bu) + 16);
sprintf(query, a, vn, bu);
statsdb->execute(query);
Expand Down
23 changes: 23 additions & 0 deletions lib/sqlite3db.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,26 @@ int SQLite3DB::prepare_v2(const char *str, sqlite3_stmt **statement) {
return rc;
}

void stmt_deleter_t::operator()(sqlite3_stmt* x) const {
proxy_sqlite3_finalize(x);
}

std::pair<int,stmt_unique_ptr> SQLite3DB::prepare_v2(const char* query) {
int rc { 0 };
sqlite3_stmt* stmt { nullptr };

do {
rc = (*proxy_sqlite3_prepare_v2)(db, query, -1, &stmt, nullptr);

if (rc==SQLITE_LOCKED || rc==SQLITE_BUSY) {
struct timespec ts { .tv_sec = 0, .tv_nsec = USLEEP_SQLITE_LOCKED * 1000 };
nanosleep(&ts, nullptr);
}
} while (rc==SQLITE_LOCKED || rc==SQLITE_BUSY);

return { rc, stmt_unique_ptr(stmt) };
}

/**
* @brief Executes a SQL statement and returns the result set.
*
Expand Down Expand Up @@ -988,6 +1008,7 @@ void SQLite3DB::LoadPlugin(const char *plugin_name) {
proxy_sqlite3_get_autocommit = NULL;
proxy_sqlite3_free = NULL;
proxy_sqlite3_status = NULL;
proxy_sqlite3_status64 = NULL;
proxy_sqlite3_changes = NULL;
proxy_sqlite3_step = NULL;
proxy_sqlite3_shutdown = NULL;
Expand Down Expand Up @@ -1066,6 +1087,7 @@ void SQLite3DB::LoadPlugin(const char *plugin_name) {
proxy_sqlite3_get_autocommit = sqlite3_get_autocommit;
proxy_sqlite3_free = sqlite3_free;
proxy_sqlite3_status = sqlite3_status;
proxy_sqlite3_status64 = sqlite3_status64;
proxy_sqlite3_changes = sqlite3_changes;
proxy_sqlite3_step = sqlite3_step;
proxy_sqlite3_shutdown = sqlite3_shutdown;
Expand Down Expand Up @@ -1094,6 +1116,7 @@ void SQLite3DB::LoadPlugin(const char *plugin_name) {
assert(proxy_sqlite3_get_autocommit);
assert(proxy_sqlite3_free);
assert(proxy_sqlite3_status);
assert(proxy_sqlite3_status64);
assert(proxy_sqlite3_changes);
assert(proxy_sqlite3_step);
assert(proxy_sqlite3_shutdown);
Expand Down