Merge pull request #705 from kamipo/fixes_694

Avoid GC run between mysql_stmt_execute and mysql_stmt_store_result

Author: sodabrew

ext/mysql2/client.c

@@ -259,7 +259,7 @@ static VALUE allocate(VALUE klass) {
   mysql_client_wrapper * wrapper;
   obj = Data_Make_Struct(klass, mysql_client_wrapper, rb_mysql_client_mark, rb_mysql_client_free, wrapper);
   wrapper->encoding = Qnil;
-  MARK_CONN_INACTIVE(self);
+  wrapper->active_thread = Qnil;
   wrapper->automatic_close = 1;
   wrapper->server_version = 0;
   wrapper->reconnect_enabled = 0;
@@ -373,7 +373,7 @@ static VALUE rb_connect(VALUE self, VALUE user, VALUE pass, VALUE host, VALUE po
     if (wrapper->connect_timeout)
       mysql_options(wrapper->client, MYSQL_OPT_CONNECT_TIMEOUT, &wrapper->connect_timeout);
     if (rv == Qfalse)
-      return rb_raise_mysql2_error(wrapper);
+      rb_raise_mysql2_error(wrapper);
   }
 
   wrapper->server_version = mysql_get_server_version(wrapper->client);
@@ -418,8 +418,8 @@ static VALUE do_send_query(void *args) {
   mysql_client_wrapper *wrapper = query_args->wrapper;
   if ((VALUE)rb_thread_call_without_gvl(nogvl_send_query, args, RUBY_UBF_IO, 0) == Qfalse) {
     /* an error occurred, we're not active anymore */
-    MARK_CONN_INACTIVE(self);
-    return rb_raise_mysql2_error(wrapper);
+    wrapper->active_thread = Qnil;
+    rb_raise_mysql2_error(wrapper);
   }
   return Qnil;
 }
@@ -448,7 +448,7 @@ static void *nogvl_do_result(void *ptr, char use_result) {
 
   /* once our result is stored off, this connection is
      ready for another command to be issued */
-  MARK_CONN_INACTIVE(self);
+  wrapper->active_thread = Qnil;
 
   return result;
 }
@@ -480,8 +480,8 @@ static VALUE rb_mysql_client_async_result(VALUE self) {
   REQUIRE_CONNECTED(wrapper);
   if ((VALUE)rb_thread_call_without_gvl(nogvl_read_query_result, wrapper->client, RUBY_UBF_IO, 0) == Qfalse) {
     /* an error occurred, mark this connection inactive */
-    MARK_CONN_INACTIVE(self);
-    return rb_raise_mysql2_error(wrapper);
+    wrapper->active_thread = Qnil;
+    rb_raise_mysql2_error(wrapper);
   }
 
   is_streaming = rb_hash_aref(rb_iv_get(self, "@current_query_options"), sym_stream);
@@ -493,7 +493,7 @@ static VALUE rb_mysql_client_async_result(VALUE self) {
 
   if (result == NULL) {
     if (mysql_errno(wrapper->client) != 0) {
-      MARK_CONN_INACTIVE(self);
+      wrapper->active_thread = Qnil;
       rb_raise_mysql2_error(wrapper);
     }
     /* no data and no error, so query was not a SELECT */
@@ -517,7 +517,7 @@ struct async_query_args {
 static VALUE disconnect_and_raise(VALUE self, VALUE error) {
   GET_CLIENT(self);
 
-  MARK_CONN_INACTIVE(self);
+  wrapper->active_thread = Qnil;
   wrapper->connected = 0;
 
   /* Invalidate the MySQL socket to prevent further communication.
@@ -588,7 +588,7 @@ static VALUE finish_and_mark_inactive(void *args) {
     result = (MYSQL_RES *)rb_thread_call_without_gvl(nogvl_store_result, wrapper, RUBY_UBF_IO, 0);
     mysql_free_result(result);
 
-    MARK_CONN_INACTIVE(self);
+    wrapper->active_thread = Qnil;
   }
 
   return Qnil;
@@ -1011,10 +1011,10 @@ static VALUE rb_mysql_client_ping(VALUE self) {
 static VALUE rb_mysql_client_more_results(VALUE self)
 {
   GET_CLIENT(self);
-    if (mysql_more_results(wrapper->client) == 0)
-      return Qfalse;
-    else
-      return Qtrue;
+  if (mysql_more_results(wrapper->client) == 0)
+    return Qfalse;
+  else
+    return Qtrue;
 }
 
 /* call-seq:
@@ -1228,7 +1228,7 @@ static VALUE initialize_ext(VALUE self) {
 
   if ((VALUE)rb_thread_call_without_gvl(nogvl_init, wrapper, RUBY_UBF_IO, 0) == Qfalse) {
     /* TODO: warning - not enough memory? */
-    return rb_raise_mysql2_error(wrapper);
+    rb_raise_mysql2_error(wrapper);
   }
 
   wrapper->initialized = 1;

ext/mysql2/client.h

@@ -59,10 +59,6 @@ typedef struct {
 
 void rb_mysql_client_set_active_thread(VALUE self);
 
-#define MARK_CONN_INACTIVE(conn) do {\
-    wrapper->active_thread = Qnil; \
-  } while(0)
-
 #define GET_CLIENT(self) \
   mysql_client_wrapper *wrapper; \
   Data_Get_Struct(self, mysql_client_wrapper, wrapper);

ext/mysql2/statement.c

@@ -11,7 +11,6 @@ static VALUE intern_usec, intern_sec, intern_min, intern_hour, intern_day, inter
   if (!stmt_wrapper->stmt) { rb_raise(cMysql2Error, "Invalid statement handle"); } \
   if (stmt_wrapper->closed) { rb_raise(cMysql2Error, "Statement handle already closed"); }
 
-
 static void rb_mysql_stmt_mark(void * ptr) {
   mysql_stmt_wrapper *stmt_wrapper = ptr;
   if (!stmt_wrapper) return;
@@ -42,7 +41,6 @@ void decr_mysql2_stmt(mysql_stmt_wrapper *stmt_wrapper) {
   }
 }
 
-
 void rb_raise_mysql2_stmt_error(mysql_stmt_wrapper *stmt_wrapper) {
   VALUE e;
   GET_CLIENT(stmt_wrapper->client);
@@ -71,7 +69,6 @@ void rb_raise_mysql2_stmt_error(mysql_stmt_wrapper *stmt_wrapper) {
   rb_exc_raise(e);
 }
 
-
 /*
  * used to pass all arguments to mysql_stmt_prepare while inside
  * nogvl_prepare_statement_args
@@ -180,16 +177,6 @@ static void *nogvl_execute(void *ptr) {
   }
 }
 
-static void *nogvl_stmt_store_result(void *ptr) {
-  MYSQL_STMT *stmt = ptr;
-
-  if (mysql_stmt_store_result(stmt)) {
-    return (void *)Qfalse;
-  } else {
-    return (void *)Qtrue;
-  }
-}
-
 static void set_buffer_for_string(MYSQL_BIND* bind_buffer, unsigned long *length_buffer, VALUE string) {
   int length;
 
@@ -369,8 +356,7 @@ static VALUE execute(int argc, VALUE *argv, VALUE self) {
   if (metadata == NULL) {
     if (mysql_stmt_errno(stmt) != 0) {
       // either CR_OUT_OF_MEMORY or CR_UNKNOWN_ERROR. both fatal.
-
-      MARK_CONN_INACTIVE(stmt_wrapper->client);
+      wrapper->active_thread = Qnil;
       rb_raise_mysql2_stmt_error(stmt_wrapper);
     }
     // no data and no error, so query was not a SELECT
@@ -384,11 +370,11 @@ static VALUE execute(int argc, VALUE *argv, VALUE self) {
   is_streaming = (Qtrue == rb_hash_aref(current, sym_stream));
   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) {
+    if (mysql_stmt_store_result(stmt)) {
       mysql_free_result(metadata);
       rb_raise_mysql2_stmt_error(stmt_wrapper);
     }
-    MARK_CONN_INACTIVE(stmt_wrapper->client);
+    wrapper->active_thread = Qnil;
   }
 
   resultObj = rb_mysql_result_to_obj(stmt_wrapper->client, wrapper->encoding, current, metadata, self);