Hand-off GVL during connect

Author: andyundso

CHANGELOG.md

@@ -10,7 +10,8 @@
 * `TinyTds::Client.new` now accepts keyword arguments instead of a hash
 * Renamed `tds_version` and `tds_version_info` to `server_version` and `server_version_info`
 * Separate `#new` and `#connect`
-  * Instead, before running `#do`, `#execute` or `#insert`, `tiny_tds` will check if the connection is active and re-connect if needed.
+  * Instead, before running `#do`, `#execute` or `#insert`, `tiny_tds` will check if the connection is active and re-connect if needed.+
+* `#connect` hands off the GVL when communicating with the server. Fixes #421.
 
 ## 3.3.0
 

ext/tiny_tds/client.c

@@ -939,9 +939,30 @@ static VALUE rb_tinytds_identity_sql(VALUE self)
   return rb_str_new2(cwrap->identity_insert_sql);
 }
 
+// connect function, with some additions to enable handing off the GVL
+struct dbopen_args {
+  LOGINREC *login;
+  const char *dataserver;
+};
+
+struct dbuse_args {
+  DBPROCESS *  	dbproc;
+  const char *  	name;
+};
 
+static void * dbopen_without_gvl(void *ptr)
+{
+  struct dbopen_args *args = (struct dbopen_args *)ptr;
+  return dbopen(args->login, args->dataserver);
+}
 
-// TinyTds::Client (protected)
+static void *
+dbuse_without_gvl(void *ptr)
+{
+  struct dbuse_args *args = (struct dbuse_args *)ptr;
+  dbuse(args->dbproc, args->name);
+  return NULL;
+}
 
 static VALUE rb_tinytds_connect(VALUE self)
 {
@@ -1011,7 +1032,19 @@ static VALUE rb_tinytds_connect(VALUE self)
     DBSETLUTF16(cwrap->login, 0);
   }
 
-  cwrap->client = dbopen(cwrap->login, StringValueCStr(dataserver));
+  // we normally have our nice NOGVL_DBCALL macro, but "dbopen" requires different arguments
+  struct dbopen_args open_args;
+  open_args.login = cwrap->login;
+  open_args.dataserver = StringValueCStr(dataserver);
+
+  cwrap->client = (DBPROCESS *)rb_thread_call_without_gvl(
+                    dbopen_without_gvl,
+                    &open_args,
+                    // compared to any other database call, we cannot cancel the login to the database
+                    // in any meaningful way, so unblock function remains empty
+                    NULL,
+                    NULL
+                  );
 
   if (cwrap->client) {
     if (dbtds(cwrap->client) < 11) {
@@ -1040,7 +1073,18 @@ static VALUE rb_tinytds_connect(VALUE self)
     cwrap->userdata->closed = 0;
 
     if (!NIL_P(database) && (azure != Qtrue)) {
-      dbuse(cwrap->client, StringValueCStr(database));
+      struct dbuse_args use_args;
+      use_args.dbproc = cwrap->client;
+      use_args.name = StringValueCStr(database);
+
+      // in case of any errors, the tinytds_err_handler will be called
+      // so we do not have to check the return code here
+      rb_thread_call_without_gvl(
+        dbuse_without_gvl,
+        &use_args,
+        NULL,
+        NULL
+      );
     }
 
     transposed_encoding = rb_funcall(cTinyTdsClient, intern_transpose_iconv_encoding, 1, charset);

test/client_test.rb

@@ -200,6 +200,17 @@ class ClientTest < TinyTds::TestCase
       end
       assert_new_connections_work
     end
+
+    it "raises TinyTds exception with invalid database name" do
+      action = lambda { new_connection(database: "DOESNOTEXIST") }
+      assert_raise_tinytds_error(action) do |e|
+        assert_equal 911, e.db_error_number
+        assert_equal 16, e.severity
+        assert_equal "Database 'DOESNOTEXIST' does not exist. Make sure that the name is entered correctly.", e.message
+      end
+
+      assert_new_connections_work
+    end
   end
 
   describe "#parse_username" do