Merge pull request #447 from sodabrew/no_dangling_connections

Specs and refactor of #444 preventing dangling connections

Author: sodabrew

ext/mysql2/client.c

@@ -196,10 +196,16 @@ static void *nogvl_close(void *ptr) {
 
 static void rb_mysql_client_free(void *ptr) {
   mysql_client_wrapper *wrapper = (mysql_client_wrapper *)ptr;
+  decr_mysql2_client(wrapper);
+}
 
+void decr_mysql2_client(mysql_client_wrapper *wrapper)
+{
   wrapper->refcount--;
   if (wrapper->refcount == 0) {
-    close_connection_and_free_mysql2_client(wrapper);
+    nogvl_close(wrapper);
+    xfree(wrapper->client);
+    xfree(wrapper);
   }
 }
 
@@ -1309,12 +1315,3 @@ void init_mysql2_client() {
       LONG2NUM(CLIENT_BASIC_FLAGS));
 #endif
 }
-
-void close_connection_and_free_mysql2_client(void *ptr) {
-  mysql_client_wrapper *wrapper = (mysql_client_wrapper *)ptr;
-
-  nogvl_close(wrapper);
-  xfree(wrapper->client);
-  xfree(wrapper);
-}
-

ext/mysql2/client.h

@@ -36,9 +36,6 @@ rb_thread_call_without_gvl(
 #endif /* ! HAVE_RB_THREAD_BLOCKING_REGION */
 #endif /* ! HAVE_RB_THREAD_CALL_WITHOUT_GVL */
 
-void init_mysql2_client();
-void close_connection_and_free_mysql2_client(void *);
-
 typedef struct {
   VALUE encoding;
   VALUE active_thread; /* rb_thread_current() or Qnil */
@@ -51,4 +48,7 @@ typedef struct {
   MYSQL *client;
 } mysql_client_wrapper;
 
+void init_mysql2_client();
+void decr_mysql2_client(mysql_client_wrapper *wrapper);
+
 #endif

ext/mysql2/result.c

@@ -1,7 +1,6 @@
 #include <mysql2_ext.h>
 #include <stdint.h>
 
-#include "client.h"
 #include "mysql_enc_to_ruby.h"
 
 #ifdef HAVE_RUBY_ENCODING_H
@@ -85,10 +84,7 @@ static void rb_mysql_result_free(void *ptr) {
 
   // 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) {
-      close_connection_and_free_mysql2_client(wrapper->client_wrapper);
-    }
+    decr_mysql2_client(wrapper->client_wrapper);
   }
 
   xfree(wrapper);

spec/mysql2/client_spec.rb

@@ -104,6 +104,22 @@ def connect *args
     ssl_client.close
   end
 
+  it "should not leave dangling connections after garbage collection" do
+    GC.start
+    client = Mysql2::Client.new(DatabaseCredentials['root'])
+    before_count = client.query("SHOW STATUS LIKE 'Threads_connected'").first['Value'].to_i
+
+    10.times do
+      Mysql2::Client.new(DatabaseCredentials['root']).query('SELECT 1')
+    end
+    after_count = client.query("SHOW STATUS LIKE 'Threads_connected'").first['Value'].to_i
+    after_count.should == before_count + 10
+
+    GC.start
+    final_count = client.query("SHOW STATUS LIKE 'Threads_connected'").first['Value'].to_i
+    final_count.should == before_count
+  end
+
   it "should be able to connect to database with numeric-only name" do
     lambda {
       creds = DatabaseCredentials['numericuser']