Merge pull request #381 from sodabrew/client-refcount

Add two refactor commits to the refcount branch

Author: brianmario

ext/mysql2/client.c

@@ -182,15 +182,14 @@ static VALUE nogvl_close(void *ptr) {
   return Qnil;
 }
 
-static void rb_mysql_client_free(void * ptr) {
+static void rb_mysql_client_free(void *ptr) {
   mysql_client_wrapper *wrapper = (mysql_client_wrapper *)ptr;
 
-  wrapper->freed = 1;
-  nogvl_close(wrapper);
-
+  wrapper->refcount--;
   if (wrapper->refcount == 0) {
+    nogvl_close(wrapper);
     xfree(wrapper->client);
-    xfree(ptr);
+    xfree(wrapper);
   }
 }
 
@@ -203,8 +202,7 @@ static VALUE allocate(VALUE klass) {
   wrapper->reconnect_enabled = 0;
   wrapper->connected = 0; /* means that a database connection is open */
   wrapper->initialized = 0; /* means that that the wrapper is initialized */
-  wrapper->refcount = 0;
-  wrapper->freed = 0;
+  wrapper->refcount = 1;
   wrapper->client = (MYSQL*)xmalloc(sizeof(MYSQL));
   return obj;
 }
@@ -366,9 +364,6 @@ static VALUE nogvl_use_result(void *ptr) {
 static VALUE rb_mysql_client_async_result(VALUE self) {
   MYSQL_RES * result;
   VALUE resultObj;
-#ifdef HAVE_RUBY_ENCODING_H
-  mysql2_result_wrapper * result_wrapper;
-#endif
   GET_CLIENT(self);
 
   /* if we're not waiting on a result, do nothing */
@@ -398,14 +393,7 @@ static VALUE rb_mysql_client_async_result(VALUE self) {
     return Qnil;
   }
 
-  resultObj = rb_mysql_result_to_obj(wrapper, result);
-  /* pass-through query options for result construction later */
-  rb_iv_set(resultObj, "@query_options", rb_hash_dup(rb_iv_get(self, "@current_query_options")));
-
-#ifdef HAVE_RUBY_ENCODING_H
-  GetMysql2Result(resultObj, result_wrapper);
-  result_wrapper->encoding = wrapper->encoding;
-#endif
+  resultObj = rb_mysql_result_to_obj(self, wrapper->encoding, rb_hash_dup(rb_iv_get(self, "@current_query_options")), result);
   return resultObj;
 }
 
@@ -931,10 +919,6 @@ static VALUE rb_mysql_client_store_result(VALUE self)
 {
   MYSQL_RES * result;
   VALUE resultObj;
-#ifdef HAVE_RUBY_ENCODING_H
-  mysql2_result_wrapper * result_wrapper;
-#endif
-
   GET_CLIENT(self);
 
   result = (MYSQL_RES *)rb_thread_blocking_region(nogvl_store_result, wrapper, RUBY_UBF_IO, 0);
@@ -947,14 +931,7 @@ static VALUE rb_mysql_client_store_result(VALUE self)
     return Qnil;
   }
 
-  resultObj = rb_mysql_result_to_obj(wrapper, result);
-  /* pass-through query options for result construction later */
-  rb_iv_set(resultObj, "@query_options", rb_hash_dup(rb_iv_get(self, "@current_query_options")));
-
-#ifdef HAVE_RUBY_ENCODING_H
-  GetMysql2Result(resultObj, result_wrapper);
-  result_wrapper->encoding = wrapper->encoding;
-#endif
+  resultObj = rb_mysql_result_to_obj(self, wrapper->encoding, rb_hash_dup(rb_iv_get(self, "@current_query_options")), result);
   return resultObj;
 
 }

ext/mysql2/result.c

@@ -64,27 +64,33 @@ static void rb_mysql_result_mark(void * wrapper) {
     rb_gc_mark(w->fields);
     rb_gc_mark(w->rows);
     rb_gc_mark(w->encoding);
+    rb_gc_mark(w->client);
   }
 }
 
 /* this may be called manually or during GC */
 static void rb_mysql_result_free_result(mysql2_result_wrapper * wrapper) {
   if (wrapper && wrapper->resultFreed != 1) {
+    /* FIXME: this may call flush_use_result, which can hit the socket */
     mysql_free_result(wrapper->result);
     wrapper->resultFreed = 1;
+  }
+}
+
+/* this is called during GC */
+static void rb_mysql_result_free(void *ptr) {
+  mysql2_result_wrapper * wrapper = ptr;
+  rb_mysql_result_free_result(wrapper);
+
+  // If the GC gets to client first it will be nil
+  if (wrapper->client != Qnil) {
     wrapper->client_wrapper->refcount--;
-    if (wrapper->client_wrapper->refcount == 0 && wrapper->client_wrapper->freed) {
+    if (wrapper->client_wrapper->refcount == 0) {
       xfree(wrapper->client_wrapper->client);
       xfree(wrapper->client_wrapper);
     }
   }
-}
 
-/* this is called during GC */
-static void rb_mysql_result_free(void * wrapper) {
-  mysql2_result_wrapper * w = wrapper;
-  /* FIXME: this may call flush_use_result, which can hit the socket */
-  rb_mysql_result_free_result(w);
   xfree(wrapper);
 }
 
@@ -567,7 +573,7 @@ static VALUE rb_mysql_result_count(VALUE self) {
 }
 
 /* Mysql2::Result */
-VALUE rb_mysql_result_to_obj(mysql_client_wrapper *client_wrapper, MYSQL_RES *r) {
+VALUE rb_mysql_result_to_obj(VALUE client, VALUE encoding, VALUE options, MYSQL_RES *r) {
   VALUE obj;
   mysql2_result_wrapper * wrapper;
   obj = Data_Make_Struct(cMysql2Result, mysql2_result_wrapper, rb_mysql_result_mark, rb_mysql_result_free, wrapper);
@@ -578,11 +584,16 @@ VALUE rb_mysql_result_to_obj(mysql_client_wrapper *client_wrapper, MYSQL_RES *r)
   wrapper->result = r;
   wrapper->fields = Qnil;
   wrapper->rows = Qnil;
-  wrapper->encoding = Qnil;
+  wrapper->encoding = encoding;
   wrapper->streamingComplete = 0;
-  wrapper->client_wrapper = client_wrapper;
+  wrapper->client = client;
+  wrapper->client_wrapper = DATA_PTR(client);
   wrapper->client_wrapper->refcount++;
+
   rb_obj_call_init(obj, 0, NULL);
+
+  rb_iv_set(obj, "@query_options", options);
+
   return obj;
 }
 

ext/mysql2/result.h

@@ -2,11 +2,12 @@
 #define MYSQL2_RESULT_H
 
 void init_mysql2_result();
-VALUE rb_mysql_result_to_obj(mysql_client_wrapper *client_wrapper, MYSQL_RES * r);
+VALUE rb_mysql_result_to_obj(VALUE client, VALUE encoding, VALUE options, MYSQL_RES *r);
 
 typedef struct {
   VALUE fields;
   VALUE rows;
+  VALUE client;
   VALUE encoding;
   unsigned int numberOfFields;
   unsigned long numberOfRows;