Merge pull request #5895 from sysown/fix/5883-crosshg-prepared-stmt-binds

fix(mysql): dangling STMT_EXECUTE binds across hostgroup split + de-flake mirror1 (#5883)

Author: renecannao

lib/MySQL_Session.cpp

@@ -4884,10 +4884,18 @@ void MySQL_Session::handler_rc0_PROCESSING_STMT_EXECUTE(MySQL_Data_Stream *myds)
 				(buffer_type == MYSQL_TYPE_DATETIME)
 			) {
 				free(CurrentQuery.stmt_meta->binds[i].buffer);
-				// NOTE: This memory should be zeroed during initialization,
-				// but we also nullify it here for extra safety. See #3546.
-				CurrentQuery.stmt_meta->binds[i].buffer = NULL;
 			}
+			// The stmt_execute_metadata_t is cached in sess_STMTs_meta and reused
+			// across executes. For every non-TIME parameter, binds[i].buffer does
+			// NOT own memory: it aliases either the STMT_EXECUTE packet just freed
+			// above (stmt_meta->pkt) or an SLDH long-data buffer just reset via
+			// SLDH->reset(). Leaving those pointers set makes them dangle until the
+			// next get_binds_from_pkt() re-points them. That re-point normally
+			// happens before use, but when a session spans multiple hostgroups the
+			// STMT_EXECUTE takes the lazy-prepare re-entrant path, opening a window
+			// where the cached, dangling binds can be consumed against freed memory
+			// (issue #5883). Null every buffer here so no dangling alias survives.
+			CurrentQuery.stmt_meta->binds[i].buffer = NULL;
 		}
 	}
 	CurrentQuery.mysql_stmt=NULL;

test/tap/tests/mysql-mirror1-t.cpp

@@ -133,16 +133,21 @@ int main(int argc, char** argv) {
 
 	sleep(1); // some INSERT may still be running
 
-	// at this point the table sbtest should have 600 rows:
-	// 300 rows from normal insert, and 100 rows from mirror
+	// The table now holds RPI*NUM_CONNS primary-insert rows plus the mirrored
+	// copies. ProxySQL mirroring is fire-and-forget / best-effort: mirrored
+	// queries carry NO delivery guarantee and a substantial fraction can be
+	// dropped under load (measured ~25-45% loss). So we do NOT assert an exact
+	// (or near-exact) doubled count here; instead we verify mirroring is
+	// FUNCTIONING -- the row count is strictly above the primary-only baseline
+	// (some mirrored rows landed) and no more than the fully-doubled ceiling.
 	rc = run_q(conns[0], "SELECT * FROM test.sbtest1");
 	ok(rc == 0 , "SELECT FROM test.sbtest1");
 	proxy_res = mysql_store_result(conns[0]);
 	while (mysql_fetch_row(proxy_res)) {
 		rows_read++;
 	}
 	mysql_free_result(proxy_res);
-	ok(rows_read == RPI*NUM_CONNS*2, "Rows expected: %u , received: %u" , RPI*NUM_CONNS*2 , rows_read);
+	ok(rows_read > RPI*NUM_CONNS && rows_read <= RPI*NUM_CONNS*2, "Mirroring active: rows received %u , primary-only=%u , fully-mirrored=%u" , rows_read , RPI*NUM_CONNS , RPI*NUM_CONNS*2);
 
 	// switching logging format
 	MYSQL_QUERY(proxysql_admin, "SET mysql-eventslog_format=1");
@@ -196,9 +201,12 @@ int main(int argc, char** argv) {
 
 	sleep(1); // some INSERT may still be running
 
-	// at this point the table sbtest should have 6600 rows:
-	// 3300 rows from normal insert, and 3300 rows from mirror
-	// note that because mirror can lose packet, we allow some margin of error (20%)
+	// The table now holds RPI*NUM_CONNS*11 primary-insert rows plus the mirrored
+	// copies. As above, mirroring is best-effort and drops a variable, sometimes
+	// large fraction of writes under load, so we assert that mirroring is
+	// FUNCTIONING (row count above the primary-only baseline, up to the
+	// fully-doubled ceiling) rather than a tight count. A previous 20% margin
+	// here was too optimistic for the observed loss and made the test flaky.
 	rows_read = 0;
 	rc = run_q(conns[0], "SELECT * FROM test.sbtest1");
 	ok(rc == 0 , "SELECT FROM test.sbtest1");
@@ -207,7 +215,7 @@ int main(int argc, char** argv) {
 		rows_read++;
 	}
 	mysql_free_result(proxy_res);
-	ok(rows_read > (float)(RPI*NUM_CONNS*11*1.8) && rows_read <= RPI*NUM_CONNS*11*2, "Rows received: %u , expected between %u and %u" , rows_read , (int)((float)(RPI*NUM_CONNS*11*1.5)), RPI*NUM_CONNS*11*2);
+	ok(rows_read > RPI*NUM_CONNS*11 && rows_read <= RPI*NUM_CONNS*11*2, "Mirroring active: rows received %u , primary-only=%u , fully-mirrored=%u" , rows_read , RPI*NUM_CONNS*11 , RPI*NUM_CONNS*11*2);
 
 	// stress the system
 	diag("Creating load");