Merge pull request #5006 from sysown/v3.0-4976

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

Author: renecannao

include/sqlite3db.h

@@ -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
@@ -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, ...);
@@ -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, ...);
@@ -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;
@@ -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 *);
 };
 

lib/ProxySQL_Admin_Stats.cpp

@@ -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
@@ -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;
@@ -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);
@@ -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 {
@@ -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);
 
@@ -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);
 	}
 
@@ -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);

lib/sqlite3db.cpp

@@ -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.
  * 
@@ -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;
@@ -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;
@@ -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);