Details
-
Bug
-
Status: Open (View Workflow)
-
Major
-
Resolution: Unresolved
-
None
-
None
-
None
Description
This issue is similar to CONC-683 whereby if "mysql_stmt_close" was called on a statement (say stmt2) while another statement (say stmt1) had been executed then the mysql_stmt_close (on stmt2) would cause the mariadb client to loose sync with the mysql network protocol. However, CONC-683 only fixed the scenario where stmt1 had not started fetching rows (state MYSQL_STMT_WAITING_USE_OR_STORE). If stmt1 is in the middle of a fetch (state MYSQL_STMT_USER_FETCHING) then the issue reported in CONC-683 still occurs.
Now, this could be fixed the same way CONC-683 was fixed. However, I would vouch for a different approach. Due to CONC-683 the code will now return early from "mysql_stmt_close" with an "out of sync" error. The problem I see with that is that the user will then be expected to re-issue that "mysql_stmt_close" at a later stage; at a point where an "out of sync" error would not occur. The absence of calling "mysql_stmt_close" again would result in memory leaks and I don't think users would expect to have to retry a "mysql_stmt_close" again if the first one failed.
On a side note, I have had a look at the MySQL sources to see how they deal with this issue and what they do is flush the active results (consume them all) regardless of which stmt they are for. Then they free the stmt passed as argument in the "mysql_stmt_close" which will make sure no leaks will occur. Can a similar approach be implemented here?
Adding the following test in ps_bugs.c will reproduce the issue:
static int test_conc_josep(MYSQL *mysql)
|
{
|
MYSQL_STMT *stmt1, *stmt2;
|
MYSQL_BIND bind;
|
int buffer;
|
int rc;
|
MYSQL_RES *result;
|
|
|
stmt1= mysql_stmt_init(mysql);
|
stmt2= mysql_stmt_init(mysql);
|
|
|
rc= mysql_stmt_prepare(stmt1, "SELECT 1 UNION SELECT 2", -1);
|
check_stmt_rc(rc, stmt1);
|
|
|
rc= mysql_stmt_execute(stmt1);
|
check_stmt_rc(rc, stmt1);
|
|
|
memset(&bind, 0, sizeof(MYSQL_BIND));
|
bind.buffer_type= MYSQL_TYPE_LONG;
|
bind.buffer= &buffer;
|
bind.buffer_length= sizeof(double);
|
|
|
rc= mysql_stmt_execute(stmt1);
|
check_stmt_rc(rc, stmt1);
|
|
|
//At this point stmt1 is in MYSQL_STMT_WAITING_USE_OR_STORE state. Thanks to the fix in CONC-683 now
|
//if we call "mysql_stmt_close(stmt2)" madb_have_pending_results() will detect stmt1 has pending results
|
//and it will thus stop it from consuming net packets (and thus losing sync with the mysql network protocol).
|
//rc= mysql_stmt_close(stmt2);
|
//FAIL_IF(!rc || mysql_stmt_errno(stmt2) != CR_COMMANDS_OUT_OF_SYNC,
|
// "Expected commands out of sync error");
|
|
|
rc= mysql_stmt_bind_result(stmt1, &bind);
|
check_stmt_rc(rc, stmt1);
|
|
|
rc= mysql_stmt_fetch(stmt1);
|
check_stmt_rc(rc, stmt1);
|
|
|
diag("buffer = %d", buffer);
|
|
|
//At this point stmt1 is in MYSQL_STMT_USER_FETCHING. This time madb_have_pending_results() won't detect
|
//stmt1 has pending results and thus it ends up calling mysql_stmt_internal_reset() which will make the
|
//next "prepare" to go out of sync with the network protocol.
|
|
|
rc= mysql_stmt_close(stmt2);
|
//FAIL_IF(!rc || mysql_stmt_errno(stmt2) != CR_COMMANDS_OUT_OF_SYNC,
|
// "Expected commands out of sync error");
|
|
|
rc= mysql_stmt_prepare(stmt2, "SELECT 1", -1);
|
check_stmt_rc(rc, stmt2);
|
|
|
result= mysql_stmt_result_metadata(stmt2);
|
FAIL_IF(!result, "Invalid result set");
|
|
|
rc= mysql_stmt_close(stmt2);
|
FAIL_IF(!rc || mysql_stmt_errno(stmt2) != CR_COMMANDS_OUT_OF_SYNC,
|
"Expected commands out of sync error");
|
|
|
rc= mysql_stmt_close(stmt1);
|
check_stmt_rc(rc, stmt1);
|
rc= mysql_stmt_close(stmt2);
|
check_stmt_rc(rc, stmt2);
|
|
|
return OK;
|
}
|