Open burnison opened 2 years ago
I was reading through the code, and I can see how this would be an issue. I'm just going to dump some thoughts here, but I don't have any ideas for how to fix this yet.
I only have a couple ideas for dealing with this right now.
I think the first option is a lot of work. Second option is probably less work, but forces the end user to change their code. Still trying to think if there is a better 3rd option.
Context
Several bugs in this repository, such as #1019, #694, and #1043, seem to relate to a common issue, which the work done 3 years ago in the unmerged #1048 appears to propose a potential solution. As noted in #1048, there is a race condition between the garbage collector and the user code that can lead to several different, unexpected errors in specs and production code. I am opening this issue to try associating them all with the same cause, to provide a meaningful work-around for library users, and to provide test cases for a potential fix.
Bugs
Ambiguous
Mysql2::Error
sInitially, we had been experiencing behaviour similar to #1019, wherein seemingly-random rspec tests would fail with an ambiguous,
Mysql2::Error
with no error message. In most cases, the top of the stack trace would point toaffected_rows
within ActiveRecord, but could also point to any other functions that perform a query.Eventually, this specific error was traced down to
rb_mysql_client_affected_rows
not expecting a return of-1
when callingmysql_affected_rows
. However, the return of-1
is being set bycli_advanced_command
with command 25,COM_STMT_CLOSE
, which is called through a macro (stmt_command
) inmysql_stmt_close
bynogvl_stmt_close
fromrb_mysql_stmt_free
during garbage collection.This specific problem can be reproduced deterministically with the following spec:
Running this spec will produce the described error:
Commands out of sync errors
Similar to #1043 and #694, this same race condition may trigger a
CR_COMMANDS_OUT_OF_SYNC
(error2014
,Commands out of sync; you can't run this command now
) between the invocation ofcli_read_query_result
andmysql_store_result
. Upon the completion ofcli_read_query_result
,mysql->status
is set toMYSQL_STATUS_GET_RESULT
, which is whatmysql_store_result
expects.However, due to the the closure of the prepared statement,
mysql->status
is set toMYSQL_STATUS_READY
inmysql_stmt_close
, preventing the completion ofmysql_store_result
and triggering error2014
.This one is harder to write a spec for, but can be deterministically proven with the following code change:
Running the test suite will reproduce this bug:
Current work-arounds
Users of
mysql2
can stop this bug occurring by ensuring that all prepared statements are explicitly closed once they're done being used --- that is, never letting a prepared statement get closed by the garbage collector.This prevents the issue due to the guard on
stmt_wrapper->stmt
and the subsequent nullification during the call tonogvl_stmt_close
. Once a statement is closed in user code, the garbage collector does not make another call to MySQL.