Support result.free

Fixes #690. When calling `mysql_stmt_execute`, we should not call `mysql_stmt_close` before calling `mysql_stmt_free_result`.
brianmario · Oct 18, 2015 · 14986f9 · 14986f9
1 parent 45ba8d9
commit 14986f9
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 13 deletions.
diff --git a/ext/mysql2/result.c b/ext/mysql2/result.c
@@ -48,7 +48,7 @@ static rb_encoding *binaryEncoding;
 #define MYSQL2_MIN_TIME 62171150401ULL
 #endif
 
-#define GET_RESULT(obj) \
+#define GET_RESULT(self) \
   mysql2_result_wrapper *wrapper; \
   Data_Get_Struct(self, mysql2_result_wrapper, wrapper);
 
@@ -86,7 +86,7 @@ static void rb_mysql_result_mark(void * wrapper) {
 }
 
 /* this may be called manually or during GC */
-static void rb_mysql_result_free_result(mysql2_result_wrapper * wrapper) {
+static void rb_mysql_result_free_result_(mysql2_result_wrapper * wrapper) {
   if (!wrapper) return;
 
   if (wrapper->resultFreed != 1) {
@@ -102,6 +102,10 @@ static void rb_mysql_result_free_result(mysql2_result_wrapper * wrapper) {
        */
       wrapper->stmt_wrapper->stmt->bind_result_done = 0;
 
+      if (wrapper->statement != Qnil) {
+        decr_mysql2_stmt(wrapper->stmt_wrapper);
+      }
+
       if (wrapper->result_buffers) {
         unsigned int i;
         for (i = 0; i < wrapper->numberOfFields; i++) {
@@ -127,20 +131,22 @@ static void rb_mysql_result_free_result(mysql2_result_wrapper * wrapper) {
 /* this is called during GC */
 static void rb_mysql_result_free(void *ptr) {
   mysql2_result_wrapper *wrapper = ptr;
-  rb_mysql_result_free_result(wrapper);
+  rb_mysql_result_free_result_(wrapper);
 
   // If the GC gets to client first it will be nil
   if (wrapper->client != Qnil) {
     decr_mysql2_client(wrapper->client_wrapper);
   }
 
-  if (wrapper->statement != Qnil) {
-    decr_mysql2_stmt(wrapper->stmt_wrapper);
-  }
-
   xfree(wrapper);
 }
 
+static VALUE rb_mysql_result_free_result(VALUE self) {
+  GET_RESULT(self);
+  rb_mysql_result_free_result_(wrapper);
+  return Qnil;
+}
+
 /*
  * for small results, this won't hit the network, but there's no
  * reliable way for us to tell this so we'll always release the GVL
@@ -792,7 +798,7 @@ static VALUE rb_mysql_result_each_(VALUE self,
         }
       } while(row != Qnil);
 
-      rb_mysql_result_free_result(wrapper);
+      rb_mysql_result_free_result_(wrapper);
       wrapper->streamingComplete = 1;
 
       // Check for errors, the connection might have gone out from under us
@@ -830,7 +836,7 @@ static VALUE rb_mysql_result_each_(VALUE self,
 
         if (row == Qnil) {
           /* we don't need the mysql C dataset around anymore, peace it */
-          rb_mysql_result_free_result(wrapper);
+          rb_mysql_result_free_result_(wrapper);
           return Qnil;
         }
 
@@ -840,7 +846,7 @@ static VALUE rb_mysql_result_each_(VALUE self,
       }
       if (wrapper->lastRowProcessed == wrapper->numberOfRows) {
         /* we don't need the mysql C dataset around anymore, peace it */
-        rb_mysql_result_free_result(wrapper);
+        rb_mysql_result_free_result_(wrapper);
       }
     }
   }
@@ -1004,6 +1010,7 @@ void init_mysql2_result() {
   cMysql2Result = rb_define_class_under(mMysql2, "Result", rb_cObject);
   rb_define_method(cMysql2Result, "each", rb_mysql_result_each, -1);
   rb_define_method(cMysql2Result, "fields", rb_mysql_result_fetch_fields, 0);
+  rb_define_method(cMysql2Result, "free", rb_mysql_result_free_result, 0);
   rb_define_method(cMysql2Result, "count", rb_mysql_result_count, 0);
   rb_define_alias(cMysql2Result, "size", "count");
 

diff --git a/ext/mysql2/statement.c b/ext/mysql2/statement.c
@@ -8,7 +8,7 @@ static VALUE intern_usec, intern_sec, intern_min, intern_hour, intern_day, inter
 #define GET_STATEMENT(self) \
   mysql_stmt_wrapper *stmt_wrapper; \
   Data_Get_Struct(self, mysql_stmt_wrapper, stmt_wrapper); \
-  if (!stmt_wrapper->stmt) { rb_raise(cMysql2Error, "Invalid statement handle"); }
+  if (stmt_wrapper->closed) { rb_raise(cMysql2Error, "Invalid statement handle"); }
 
 
 static void rb_mysql_stmt_mark(void * ptr) {
@@ -104,6 +104,7 @@ VALUE rb_mysql_stmt_new(VALUE rb_client, VALUE sql) {
   rb_stmt = Data_Make_Struct(cMysql2Statement, mysql_stmt_wrapper, rb_mysql_stmt_mark, rb_mysql_stmt_free, stmt_wrapper);
   {
     stmt_wrapper->client = rb_client;
+    stmt_wrapper->closed = 0;
     stmt_wrapper->refcount = 1;
     stmt_wrapper->stmt = NULL;
   }
@@ -461,7 +462,10 @@ static VALUE rb_mysql_stmt_affected_rows(VALUE self) {
  */
 static VALUE rb_mysql_stmt_close(VALUE self) {
   GET_STATEMENT(self);
-  rb_thread_call_without_gvl(nogvl_stmt_close, stmt_wrapper, RUBY_UBF_IO, 0);
+  if (stmt_wrapper->refcount == 1) {
+    rb_thread_call_without_gvl(nogvl_stmt_close, stmt_wrapper, RUBY_UBF_IO, 0);
+  }
+  stmt_wrapper->closed = 1;
   return Qnil;
 }
 

diff --git a/ext/mysql2/statement.h b/ext/mysql2/statement.h
@@ -6,6 +6,7 @@ extern VALUE cMysql2Statement;
 typedef struct {
   VALUE client;
   MYSQL_STMT *stmt;
+  char closed;
   int refcount;
 } mysql_stmt_wrapper;
 

diff --git a/spec/mysql2/statement_spec.rb b/spec/mysql2/statement_spec.rb
@@ -669,7 +669,8 @@
 
   context 'close' do
     it 'should free server resources' do
-      stmt = @client.prepare 'SELECT 1'
+      stmt = @client.prepare 'SELECT 1 FROM dual WHERE 0 = ?'
+      stmt.execute(1).free
       expect { stmt.close }.to change {
         @client.query("SHOW STATUS LIKE 'Prepared_stmt_count'").first['Value'].to_i
       }.by(-1)