Skip to content

Commit 6f2798a

Browse files
committed
Fix memory leak in 'ProxySQL_Admin::stats___mysql_global' - Closes #4976
The resource leak was associated to prepared statements (sqlite3_stmt). Since the detection of these leaks is tricky, because the resources held by 'SQLite3' never lose their references, a helper type ('stmt_unique_ptr') and a safer variation for 'prepare_v2' have been introduced. This helper type should automatically handle the lifetime of the statements, avoiding these kind of leaks in the future. The non-safe alternative has been flagged as 'deprecated', so it can be easily identified in later refactors or slowly removed during development.
1 parent 71e4fc6 commit 6f2798a

File tree

3 files changed

+50
-4
lines changed

3 files changed

+50
-4
lines changed

include/sqlite3db.h

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
#undef min
77
#undef max
88
#include <cstdint>
9+
#include <memory>
910
#include <vector>
11+
#include <utility>
1012
#define PROXYSQL_SQLITE3DB_PTHREAD_MUTEX
1113

1214
#ifndef SAFE_SQLITE3_STEP2
@@ -165,6 +167,18 @@ class SQLite3_result {
165167
void dump_to_stderr();
166168
};
167169

170+
/**
171+
* @brief Helper type for finalizing 'sqlite3_stmt' managed by smart pointers.
172+
*/
173+
struct stmt_deleter_t {
174+
void operator()(sqlite3_stmt* x) const;
175+
};
176+
177+
/**
178+
* @brief Safe type for automatically deallocation of 'sqlite3_stmt'.
179+
*/
180+
using stmt_unique_ptr = std::unique_ptr<sqlite3_stmt, stmt_deleter_t>;
181+
168182
class SQLite3DB {
169183
private:
170184
char *url;
@@ -191,7 +205,14 @@ class SQLite3DB {
191205
int check_table_structure(char *table_name, char *table_def);
192206
bool build_table(char *table_name, char *table_def, bool dropit);
193207
bool check_and_build_table(char *table_name, char *table_def);
208+
[[deprecated("Use safer alternative 'prepare_v2(const char *)'")]]
194209
int prepare_v2(const char *, sqlite3_stmt **);
210+
/**
211+
* @brief Prepares a query as a statement in the SQLite3DB.
212+
* @param query The query to be prepared as an 'sqlite3_stmt'.
213+
* @return A pair of with shape { err_code, stmt_unique_ptr }.
214+
*/
215+
std::pair<int,stmt_unique_ptr> prepare_v2(const char* query);
195216
static void LoadPlugin(const char *);
196217
};
197218

lib/ProxySQL_Admin_Stats.cpp

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -521,12 +521,17 @@ void ProxySQL_Admin::stats___mysql_global() {
521521
"INSERT INTO stats_mysql_global VALUES " + generate_multi_rows_query(32, 2)
522522
};
523523

524-
sqlite3_stmt* row_stmt = nullptr;
525-
int rc = statsdb->prepare_v2(q_row_insert.c_str(), &row_stmt);
524+
int rc = 0;
525+
526+
stmt_unique_ptr u_row_stmt { nullptr };
527+
std::tie(rc, u_row_stmt) = statsdb->prepare_v2(q_row_insert.c_str());
526528
ASSERT_SQLITE_OK(rc, statsdb);
527-
sqlite3_stmt* bulk_stmt = nullptr;
528-
rc = statsdb->prepare_v2(q_bulk_insert.c_str(), &bulk_stmt);
529+
sqlite3_stmt* const row_stmt { u_row_stmt.get() };
530+
531+
stmt_unique_ptr u_bulk_stmt { nullptr };
532+
std::tie(rc, u_bulk_stmt) = statsdb->prepare_v2(q_bulk_insert.c_str());
529533
ASSERT_SQLITE_OK(rc, statsdb);
534+
sqlite3_stmt* const bulk_stmt { u_bulk_stmt.get() };
530535

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

lib/sqlite3db.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,26 @@ int SQLite3DB::prepare_v2(const char *str, sqlite3_stmt **statement) {
260260
return rc;
261261
}
262262

263+
void stmt_deleter_t::operator()(sqlite3_stmt* x) const {
264+
proxy_sqlite3_finalize(x);
265+
}
266+
267+
std::pair<int,stmt_unique_ptr> SQLite3DB::prepare_v2(const char* query) {
268+
int rc { 0 };
269+
sqlite3_stmt* stmt { nullptr };
270+
271+
do {
272+
rc = (*proxy_sqlite3_prepare_v2)(db, query, -1, &stmt, nullptr);
273+
274+
if (rc==SQLITE_LOCKED || rc==SQLITE_BUSY) {
275+
struct timespec ts { .tv_sec = 0, .tv_nsec = USLEEP_SQLITE_LOCKED * 1000 };
276+
nanosleep(&ts, nullptr);
277+
}
278+
} while (rc==SQLITE_LOCKED || rc==SQLITE_BUSY);
279+
280+
return { rc, stmt_unique_ptr(stmt) };
281+
}
282+
263283
/**
264284
* @brief Executes a SQL statement and returns the result set.
265285
*

0 commit comments

Comments
 (0)