Keep a reference from the Result to the Statement

This prevents the GC from getting the Statement first, which would
call mysql_stmt_close, which calls mysql_stmt_result_free on any
outstanding results. When the Result object gets GC next, it also
calls mysql_stmt_result_free and that would crash.

Author: sodabrew

ext/mysql2/client.c

@@ -490,7 +490,7 @@ static VALUE rb_mysql_client_async_result(VALUE self) {
   current = rb_hash_dup(rb_iv_get(self, "@current_query_options"));
   (void)RB_GC_GUARD(current);
   Check_Type(current, T_HASH);
-  resultObj = rb_mysql_result_to_obj(self, wrapper->encoding, current, result, NULL);
+  resultObj = rb_mysql_result_to_obj(self, wrapper->encoding, current, result, Qnil);
 
   return resultObj;
 }
@@ -1050,7 +1050,7 @@ static VALUE rb_mysql_client_store_result(VALUE self)
   current = rb_hash_dup(rb_iv_get(self, "@current_query_options"));
   (void)RB_GC_GUARD(current);
   Check_Type(current, T_HASH);
-  resultObj = rb_mysql_result_to_obj(self, wrapper->encoding, current, result, NULL);
+  resultObj = rb_mysql_result_to_obj(self, wrapper->encoding, current, result, Qnil);
 
   return resultObj;
 }

ext/mysql2/result.c

@@ -75,26 +75,28 @@ static VALUE sym_symbolize_keys, sym_as, sym_array, sym_database_timezone, sym_a
           sym_local, sym_utc, sym_cast_booleans, sym_cache_rows, sym_cast, sym_stream, sym_name;
 static ID intern_merge;
 
+/* Mark any VALUEs that are only referenced in C, so the GC won't get them. */
 static void rb_mysql_result_mark(void * wrapper) {
   mysql2_result_wrapper * w = wrapper;
   if (w) {
     rb_gc_mark(w->fields);
     rb_gc_mark(w->rows);
     rb_gc_mark(w->encoding);
     rb_gc_mark(w->client);
+    rb_gc_mark(w->statement);
   }
 }
 
 /* this may be called manually or during GC */
 static void rb_mysql_result_free_result(mysql2_result_wrapper * wrapper) {
-  unsigned int i;
   if (!wrapper) return;
 
   if (wrapper->resultFreed != 1) {
     if (wrapper->stmt) {
       mysql_stmt_free_result(wrapper->stmt);
 
       if (wrapper->result_buffers) {
+        unsigned int i;
         for (i = 0; i < wrapper->numberOfFields; i++) {
           if (wrapper->result_buffers[i].buffer) {
             xfree(wrapper->result_buffers[i].buffer);
@@ -107,6 +109,7 @@ static void rb_mysql_result_free_result(mysql2_result_wrapper * wrapper) {
       }
     }
     /* FIXME: this may call flush_use_result, which can hit the socket */
+    /* For prepared statements, wrapper->result is the result metadata */
     mysql_free_result(wrapper->result);
     wrapper->resultFreed = 1;
   }
@@ -949,7 +952,7 @@ static VALUE rb_mysql_result_count(VALUE self) {
 }
 
 /* Mysql2::Result */
-VALUE rb_mysql_result_to_obj(VALUE client, VALUE encoding, VALUE options, MYSQL_RES *r, MYSQL_STMT * s) {
+VALUE rb_mysql_result_to_obj(VALUE client, VALUE encoding, VALUE options, MYSQL_RES *r, VALUE statement) {
   VALUE obj;
   mysql2_result_wrapper * wrapper;
 
@@ -966,12 +969,19 @@ VALUE rb_mysql_result_to_obj(VALUE client, VALUE encoding, VALUE options, MYSQL_
   wrapper->client = client;
   wrapper->client_wrapper = DATA_PTR(client);
   wrapper->client_wrapper->refcount++;
-  wrapper->stmt = s;
   wrapper->result_buffers = NULL;
   wrapper->is_null = NULL;
   wrapper->error = NULL;
   wrapper->length = NULL;
 
+  /* Keep a handle to the Statement to ensure it doesn't get garbage collected first */
+  wrapper->statement = statement;
+  if (statement != Qnil) {
+    mysql_stmt_wrapper *stmt_wrapper = DATA_PTR(statement);
+    wrapper->stmt = stmt_wrapper->stmt;
+    stmt_wrapper->refcount++;
+  }
+
   rb_obj_call_init(obj, 0, NULL);
   rb_iv_set(obj, "@query_options", options);
 

ext/mysql2/result.h

@@ -2,13 +2,14 @@
 #define MYSQL2_RESULT_H
 
 void init_mysql2_result();
-VALUE rb_mysql_result_to_obj(VALUE client, VALUE encoding, VALUE options, MYSQL_RES *r, MYSQL_STMT * s);
+VALUE rb_mysql_result_to_obj(VALUE client, VALUE encoding, VALUE options, MYSQL_RES *r, VALUE statement);
 
 typedef struct {
   VALUE fields;
   VALUE rows;
   VALUE client;
   VALUE encoding;
+  VALUE statement;
   unsigned int numberOfFields;
   unsigned long numberOfRows;
   unsigned long lastRowProcessed;

ext/mysql2/statement.c

@@ -359,12 +359,13 @@ static VALUE execute(int argc, VALUE *argv, VALUE self) {
   if (!is_streaming) {
     // recieve the whole result set from the server
     if (rb_thread_call_without_gvl(nogvl_stmt_store_result, stmt, RUBY_UBF_IO, 0) == Qfalse) {
+      mysql_free_result(metadata);
       rb_raise_mysql2_stmt_error(self);
     }
     MARK_CONN_INACTIVE(stmt_wrapper->client);
   }
 
-  resultObj = rb_mysql_result_to_obj(stmt_wrapper->client, wrapper->encoding, current, metadata, stmt);
+  resultObj = rb_mysql_result_to_obj(stmt_wrapper->client, wrapper->encoding, current, metadata, self);
 
   if (!is_streaming) {
     // cache all result

ext/mysql2/statement.h

@@ -8,6 +8,7 @@ void init_mysql2_statement();
 typedef struct {
   VALUE client;
   MYSQL_STMT *stmt;
+  int refcount;
 } mysql_stmt_wrapper;
 
 VALUE rb_mysql_stmt_new(VALUE rb_client, VALUE sql);