Merge pull request #3598 from sysown/v2.2.1-3597

Closes #3597: Backport bugfixes to v2.2.1 - 2

Author: JavierJF

include/MySQL_Protocol.h

@@ -64,6 +64,22 @@ class MySQL_Prepared_Stmt_info {
 
 uint8_t mysql_decode_length(unsigned char *ptr, uint64_t *len);
 
+/**
+ * @brief ProxySQL replacement function for 'mysql_stmt_close'. Closes a
+ *   MYSQL_STMT avoiding any blocking commands that are sent by default
+ *   'mysql_stmt_close'.
+ *
+ *   NOTE: This function is not safe, caller must check that the supplied
+ *   argument is not NULL.
+ *
+ * @param mysql_stmt An already initialized 'MYSQL_STMT'. Caller must ensure
+ *   that the supplied argument is not NULL.
+ *
+ * @return The result of calling 'mysql_stmt_close' function over the internally
+ *   modified 'MYSQL_STMT'.
+ */
+my_bool proxy_mysql_stmt_close(MYSQL_STMT* mysql_stmt);
+
 class MySQL_Protocol {
 	private:
 	MySQL_Connection_userinfo *userinfo;

include/gen_utils.h

@@ -229,6 +229,7 @@ inline unsigned long long realtime_time() {
   clock_gettime(CLOCK_REALTIME, &ts);
   return (((unsigned long long) ts.tv_sec) * 1000000) + (ts.tv_nsec / 1000);
 }
+
 #endif /* __GEN_FUNCTIONS */
 
 bool Proxy_file_exists(const char *);

lib/MySQL_Monitor.cpp

@@ -431,6 +431,7 @@ MySQL_Monitor_State_Data::MySQL_Monitor_State_Data(char *h, int p, struct event_
 		use_ssl=_use_ssl;
 		ST=0;
 		hostgroup_id=g;
+		interr=0;
 	};
 
 MySQL_Monitor_State_Data::~MySQL_Monitor_State_Data() {
@@ -2138,7 +2139,7 @@ void * monitor_replication_lag_thread(void *arg) {
 		int l = strlen(percona_heartbeat_table);
 		if (l) {
 			use_percona_heartbeat = true;
-			char *base_query = (char *)"SELECT MIN(ROUND(TIMESTAMPDIFF(MICROSECOND, ts, SYSDATE(6))/1000000)) AS Seconds_Behind_Master FROM %s";
+			char *base_query = (char *)"SELECT MAX(ROUND(TIMESTAMPDIFF(MICROSECOND, ts, SYSDATE(6))/1000000)) AS Seconds_Behind_Master FROM %s";
 			char *replication_query = (char *)malloc(strlen(base_query)+l);
 			sprintf(replication_query,base_query,percona_heartbeat_table);
 			mmsd->async_exit_status=mysql_query_start(&mmsd->interr,mmsd->mysql,replication_query);

lib/MySQL_PreparedStatement.cpp

@@ -4,6 +4,7 @@
 #include "SpookyV2.h"
 
 #include "MySQL_PreparedStatement.h"
+#include "MySQL_Protocol.h"
 
 //extern MySQL_STMT_Manager *GloMyStmt;
 //static uint32_t add_prepared_statement_calls = 0;
@@ -691,12 +692,7 @@ MySQL_STMTs_local_v14::~MySQL_STMTs_local_v14() {
 			it != global_stmt_to_backend_stmt.end(); ++it) {
 			uint64_t global_stmt_id = it->first;
 			MYSQL_STMT *stmt = it->second;
-			if (stmt->mysql) {
-				stmt->mysql->stmts =
-				    list_delete(stmt->mysql->stmts, &stmt->list);
-			}
-			stmt->mysql = NULL;
-			mysql_stmt_close(stmt);
+			proxy_mysql_stmt_close(stmt);
 			GloMyStmt->ref_count_server(global_stmt_id, -1);
 		}
 	}

lib/MySQL_Protocol.cpp

@@ -2033,7 +2033,7 @@ bool MySQL_Protocol::process_pkt_handshake_response(unsigned char *pkt, unsigned
 		(*myds)->sess->session_fast_forward=fast_forward;
 		(*myds)->sess->user_max_connections=max_connections;
 	}
-	if (password==NULL) {
+	if (password == NULL) {
 		// this is a workaround for bug #603
 		if (
 			((*myds)->sess->session_type == PROXYSQL_SESSION_ADMIN)
@@ -2098,7 +2098,7 @@ bool MySQL_Protocol::process_pkt_handshake_response(unsigned char *pkt, unsigned
 						(*myds)->sess->transaction_persistent=transaction_persistent;
 						(*myds)->sess->session_fast_forward=fast_forward;
 						(*myds)->sess->user_max_connections=max_connections;
-						if (strncmp(password,(char *)pass,strlen(password))==0) {
+						if (strcmp(password, (char *) pass) == 0) {
 							if (backend_username) {
 								free(password);
 								password=NULL;
@@ -2157,8 +2157,8 @@ bool MySQL_Protocol::process_pkt_handshake_response(unsigned char *pkt, unsigned
 						ret=true;
 					}
 				} else { // mysql_clear_password
-					if (strncmp(password,(char *)pass,strlen(password))==0) {
-						ret=true;
+					if (strcmp(password, (char *) pass) == 0) {
+						ret = true;
 					}
 				}
 			} else {
@@ -3072,3 +3072,15 @@ unsigned long long MySQL_ResultSet::current_size() {
 	}
 	return intsize;
 }
+
+my_bool proxy_mysql_stmt_close(MYSQL_STMT* stmt) {
+	// Clean internal structures for 'stmt->mysql->stmts'.
+	if (stmt->mysql) {
+		stmt->mysql->stmts =
+			list_delete(stmt->mysql->stmts, &stmt->list);
+	}
+	// Nullify 'mysql' field to avoid sending a blocking command to the server.
+	stmt->mysql = NULL;
+	// Perform the regular close operation.
+	return mysql_stmt_close(stmt);
+}

lib/MySQL_Thread.cpp

@@ -2051,7 +2051,7 @@ char ** MySQL_Threads_Handler::get_variables_list() {
 		// query processor and query digest
 		VariablesPointers_int["auto_increment_delay_multiplex"]  = make_tuple(&variables.auto_increment_delay_multiplex,   0,     1000000, false);
 		VariablesPointers_int["default_query_delay"]             = make_tuple(&variables.default_query_delay,              0,   3600*1000, false);
-		VariablesPointers_int["default_query_timeout"]           = make_tuple(&variables.default_query_timeout,            0,   1000*1000, false);
+		VariablesPointers_int["default_query_timeout"]           = make_tuple(&variables.default_query_timeout,         1000,20*24*3600*1000, false);
 		VariablesPointers_int["query_digests_grouping_limit"]    = make_tuple(&variables.query_digests_grouping_limit,     1,        2089, false);
 		VariablesPointers_int["query_digests_max_digest_length"] = make_tuple(&variables.query_digests_max_digest_length, 16, 1*1024*1024, false);
 		VariablesPointers_int["query_digests_max_query_length"]  = make_tuple(&variables.query_digests_max_query_length,  16, 1*1024*1024, false);

lib/gen_utils.cpp

@@ -219,3 +219,4 @@ bool Proxy_file_regular(const char *path) {
 		if (sb.st_mode & S_IFREG) return true;
 	return false;
 }
+

lib/mysql_connection.cpp

@@ -654,7 +654,7 @@ void MySQL_Connection::connect_start() {
 		} else {
 			mysql_options4(mysql, MYSQL_OPT_CONNECT_ATTR_ADD, "proxysql_sha1", "unknown");
 		}
-		mysql_options4(mysql, MYSQL_OPT_CONNECT_ATTR_ADD, "mysql_bug_102266", "ProxySQL is sending a lot of data to MySQL server using CLIENT_CONNECT_ATTRS in order to not hit MySQL bug https://bugs.mysql.com/bug.php?id=102266 . See also https://github.com/sysown/proxysql/issues/3276");
+		mysql_options4(mysql, MYSQL_OPT_CONNECT_ATTR_ADD, "mysql_bug_102266", "Avoid MySQL bug https://bugs.mysql.com/bug.php?id=102266 , https://github.com/sysown/proxysql/issues/3276");
 	}
 	if (parent->use_ssl) {
 		mysql_ssl_set(mysql, mysql_thread___ssl_p2s_key, mysql_thread___ssl_p2s_cert, mysql_thread___ssl_p2s_ca, NULL, mysql_thread___ssl_p2s_cipher);
@@ -2144,6 +2144,15 @@ void MySQL_Connection::async_free_result() {
 					mysql_stmt_free_result(query.stmt);
 				}
 			}
+			// If we reached here from 'ASYNC_STMT_PREPARE_FAILED', the
+			// prepared statement was never added to 'local_stmts', thus
+			// it will never be freed when 'local_stmts' are purged. If
+			// initialized, it must be freed. For more context see #3525.
+			if (this->async_state_machine == ASYNC_STMT_PREPARE_FAILED) {
+				if (query.stmt != NULL) {
+					proxy_mysql_stmt_close(query.stmt);
+				}
+			}
 			query.stmt=NULL;
 		}
 		if (mysql_result) {

test/tap/tests/repro_test_leak_3525.cpp

@@ -0,0 +1,73 @@
+/**
+ * @file repro_test_leak_3525.cpp
+ * @brief Test to reproduce issue #3525.
+ * @details This test is not meant to be executed, it's just a left as DOC of how to
+ *   reproduce issue #3525.
+ * @date 2021-07-30
+ */
+
+#include <string>
+#include <mysql.h>
+
+#include "tap.h"
+#include "command_line.h"
+#include "utils.h"
+
+int main(int argc, char** argv) {
+	CommandLine cl;
+
+	if(cl.getEnv())
+		return exit_status();
+
+	plan(1);
+
+	MYSQL* mysql = mysql_init(NULL);
+	if (mysql == NULL) {
+		return exit_status();
+	}
+
+	if (
+		!mysql_real_connect(mysql, cl.host, cl.username, cl.password, NULL, cl.port, NULL, 0)
+	) {
+		fprintf(stderr, "Failed to connect to database: Error: %s\n", mysql_error(mysql));
+		return exit_status();
+	}
+
+	MYSQL_STMT *stmt = mysql_stmt_init(mysql);
+	if (stmt == NULL) {
+		ok(false, " mysql_stmt_init(), out of memory\n");
+		return exit_status();
+	}
+
+	// Invalid statement
+	bool failed = false;
+	std::string my_errmsg {};
+	int my_errno = 0;
+
+	std::string query { "BEGIN" };
+	if (mysql_stmt_prepare(stmt, query.c_str(), query.size())) {
+		my_errno = mysql_errno(mysql);
+		my_errmsg = mysql_error(mysql);
+
+		failed = true;
+	} else {
+		my_errmsg = "NOT A FAILURE!";
+	}
+
+	ok(
+		failed, "'mysql_stmt_prepare' should fail: ('err_code': %d, 'err_msg': '%s')\n",
+		my_errno, my_errmsg.c_str()
+	);
+
+	if (mysql_stmt_close(stmt)) {
+		ok(false, "mysql_stmt_close at line %d failed: %s\n", __LINE__ , mysql_error(mysql));
+		return exit_status();
+	}
+
+	mysql_stmt_close(stmt);
+	mysql_close(mysql);
+	mysql_library_end();
+
+	mysql_close(mysql);
+}
+