Merge pull request #1 from manhnguyenvan/terminate-on-close

Fix close connection to mysql

Author: manhnguyenvan

ext/mysql2/client.c

@@ -226,20 +226,6 @@ static void *nogvl_close(void *ptr) {
   if (wrapper->connected) {
     wrapper->active_thread = Qnil;
     wrapper->connected = 0;
-#ifndef _WIN32
-    /* Invalidate the socket before calling mysql_close(). This prevents
-     * mysql_close() from sending a mysql-QUIT or from calling shutdown() on
-     * the socket. The difference is that invalidate_fd will drop this
-     * process's reference to the socket only, while a QUIT or shutdown()
-     * would render the underlying connection unusable, interrupting other
-     * processes which share this object across a fork().
-     */
-    if (invalidate_fd(wrapper->client->net.fd) == Qfalse) {
-      fprintf(stderr, "[WARN] mysql2 failed to invalidate FD safely, leaking some memory\n");
-      close(wrapper->client->net.fd);
-      return NULL;
-    }
-#endif
 
     mysql_close(wrapper->client); /* only used to free memory at this point */
   }
@@ -256,6 +242,24 @@ void decr_mysql2_client(mysql_client_wrapper *wrapper)
 {
   wrapper->refcount--;
   if (wrapper->refcount == 0) {
+
+#ifndef _WIN32
+    if (wrapper->connected) {
+      /* Invalidate the socket before calling mysql_close(). This prevents
+       * mysql_close() from sending a mysql-QUIT or from calling shutdown() on
+       * the socket. The difference is that invalidate_fd will drop this
+       * process's reference to the socket only, while a QUIT or shutdown()
+       * would render the underlying connection unusable, interrupting other
+       * processes which share this object across a fork().
+       */
+      if (invalidate_fd(wrapper->client->net.fd) == Qfalse) {
+        fprintf(stderr, "[WARN] mysql2 failed to invalidate FD safely, leaking some memory\n");
+        close(wrapper->client->net.fd);
+        return NULL;
+      }
+    }
+#endif
+
     nogvl_close(wrapper);
     xfree(wrapper->client);
     xfree(wrapper);
@@ -389,10 +393,15 @@ static VALUE rb_connect(VALUE self, VALUE user, VALUE pass, VALUE host, VALUE po
 }
 
 /*
- * Immediately disconnect from the server, normally the garbage collector
- * will disconnect automatically when a connection is no longer needed.
- * Explicitly closing this will free up server resources sooner than waiting
- * for the garbage collector.
+ * Terminate the connection; call this when the connection is no longer needed.
+ * The garbage collector can close the connection, but doing so emits an
+ * "Aborted connection" error on the server and increments the Aborted_clients
+ * status variable.
+ *
+ * @see http://dev.mysql.com/doc/en/communication-errors.html
+ * @see https://github.com/brianmario/mysql2/pull/663
+ * @return [void]
+ * 
  */
 static VALUE rb_mysql_client_close(VALUE self) {
   GET_CLIENT(self);

spec/mysql2/client_spec.rb

@@ -153,6 +153,14 @@ def connect *args
     ssl_client.close
   end
 
+  it "should terminate connections when calling close" do
+    expect {
+      Mysql2::Client.new(DatabaseCredentials['root']).close
+    }.to_not change {
+      @client.query("SHOW STATUS LIKE 'Aborted_clients'").first['Value'].to_i
+    }
+  end
+
   it "should not leave dangling connections after garbage collection" do
     GC.start
     sleep 0.300 # Let GC do its work