Skip to content

Commit 21d308f

Browse files
committed
Avoid GC run between mysql_stmt_execute and mysql_stmt_store_result
Thanks to @kamipo for diagnosing the problem and drafting the first PR. This fixes a regression caused by brianmario#912 due to calling `rb_funcall` between `mysql_stmt_execute` and `mysql_stmt_store_result`, it will cause `mysql_stmt_close` to be called in wrong order. In further testing, `rb_hash_aref` can also call `rb_funcall` if the query_options hash has a `default_proc` set, so adding a stronger comment to remind future maintainers. Fixes brianmario#956.
1 parent bf227ac commit 21d308f

File tree

1 file changed

+16
-11
lines changed

1 file changed

+16
-11
lines changed

ext/mysql2/statement.c

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,22 @@ static VALUE rb_mysql_stmt_execute(int argc, VALUE *argv, VALUE self) {
403403
}
404404
}
405405

406+
// Duplicate the options hash, merge! extra opts, put the copy into the Result object
407+
current = rb_hash_dup(rb_iv_get(stmt_wrapper->client, "@query_options"));
408+
(void)RB_GC_GUARD(current);
409+
Check_Type(current, T_HASH);
410+
411+
// Merge in hash opts/keyword arguments
412+
if (!NIL_P(opts)) {
413+
rb_funcall(current, intern_merge_bang, 1, opts);
414+
}
415+
416+
is_streaming = (Qtrue == rb_hash_aref(current, sym_stream));
417+
418+
// From stmt_execute to mysql_stmt_result_metadata to stmt_store_result,
419+
// no Ruby API calls are allowed. The GC may collect the Statement object,
420+
// and not even RB_GC_GUARD seems to be able to protect it here.
421+
406422
if ((VALUE)rb_thread_call_without_gvl(nogvl_stmt_execute, stmt, RUBY_UBF_IO, 0) == Qfalse) {
407423
FREE_BINDS;
408424
rb_raise_mysql2_stmt_error(stmt_wrapper);
@@ -421,17 +437,6 @@ static VALUE rb_mysql_stmt_execute(int argc, VALUE *argv, VALUE self) {
421437
return Qnil;
422438
}
423439

424-
// Duplicate the options hash, merge! extra opts, put the copy into the Result object
425-
current = rb_hash_dup(rb_iv_get(stmt_wrapper->client, "@query_options"));
426-
(void)RB_GC_GUARD(current);
427-
Check_Type(current, T_HASH);
428-
429-
// Merge in hash opts/keyword arguments
430-
if (!NIL_P(opts)) {
431-
rb_funcall(current, intern_merge_bang, 1, opts);
432-
}
433-
434-
is_streaming = (Qtrue == rb_hash_aref(current, sym_stream));
435440
if (!is_streaming) {
436441
// recieve the whole result set from the server
437442
if (mysql_stmt_store_result(stmt)) {

0 commit comments

Comments
 (0)