From aa8e27758c6afa23d3cf15c79fd8a1aad194490b Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Thu, 15 Oct 2026 03:17:05 +0200 Subject: [PATCH] fix(ext): raise login errors after dbopen returns The handlers raised a login error as soon as FreeTDS reported it. The raise longjmped out of dbopen() and leaked the half-open connection (70 KB to 150 KB for each failed attempt). Keep the first login error. Raise it after dbopen() returns. Close the connection before the raise when the server negotiates a TDS version older than 7.3. --- CHANGELOG.md | 2 ++ ext/tiny_tds/client.c | 84 ++++++++++++++++++++++++++++++++++++------- test/client_test.rb | 8 +++++ 3 files changed, 81 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8166f1f..7daafe85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## (unreleased) +* Raise a login error after `dbopen` returns, not from inside FreeTDS. A failed connection attempt no longer leaks the half-open connection (70 KB to 150 KB of native memory each time). Close the connection before the error when the server negotiates a TDS version older than 7.3. + ## 3.4.1 * Register the C globals that hold Ruby classes and modules with the GC, so raising `TinyTds::Error` after GC compaction no longer crashes with a segmentation fault. Fixes #608. diff --git a/ext/tiny_tds/client.c b/ext/tiny_tds/client.c index 4e84efc1..f857d576 100644 --- a/ext/tiny_tds/client.c +++ b/ext/tiny_tds/client.c @@ -66,19 +66,9 @@ static const rb_data_type_t tinytds_client_wrapper_type = { // Lib Backend (Helpers) -VALUE rb_tinytds_raise_error(DBPROCESS *dbproc, tinytds_errordata error) +static VALUE rb_tinytds_new_error(tinytds_errordata error) { - VALUE e; - GET_CLIENT_USERDATA(dbproc); - - if (error.cancel && !dbdead(dbproc) && userdata && !userdata->closed) { - userdata->dbsqlok_sent = 1; - dbsqlok(dbproc); - userdata->dbcancel_sent = 1; - dbcancel(dbproc); - } - - e = rb_exc_new2(cTinyTdsError, error.error); + VALUE e = rb_exc_new2(cTinyTdsError, error.error); rb_funcall(e, intern_source_eql, 1, rb_str_new2(error.source)); if (error.severity) { @@ -93,6 +83,23 @@ VALUE rb_tinytds_raise_error(DBPROCESS *dbproc, tinytds_errordata error) rb_funcall(e, intern_os_error_number_eql, 1, INT2FIX(error.oserr)); } + return e; +} + +VALUE rb_tinytds_raise_error(DBPROCESS *dbproc, tinytds_errordata error) +{ + VALUE e; + GET_CLIENT_USERDATA(dbproc); + + if (error.cancel && !dbdead(dbproc) && userdata && !userdata->closed) { + userdata->dbsqlok_sent = 1; + dbsqlok(dbproc); + userdata->dbcancel_sent = 1; + dbcancel(dbproc); + } + + e = rb_tinytds_new_error(error); + if (error.severity <= 10 && error.is_message) { VALUE message_handler = userdata && userdata->message_handler ? userdata->message_handler : Qnil; @@ -121,6 +128,34 @@ static void push_userdata_error(tinytds_client_userdata *userdata, tinytds_error userdata->nonblocking_errors_length++; } +/* +dbopen() calls the handlers during the login, before the DBPROCESS has its userdata. +A raise at that point longjmps out of FreeTDS and leaks the half-open connection. +Thus the handlers keep the first error here, and rb_tinytds_connect() raises it after dbopen(). +dbopen() runs with the GVL held, so only one login is in progress at a time. +*/ +typedef struct { + int occurred; + tinytds_errordata error; +} tinytds_login_error; + +static tinytds_login_error *pending_login_error = NULL; + +static int defer_login_error(tinytds_client_userdata *userdata, tinytds_errordata error) +{ + if (userdata || !pending_login_error) { + return 0; + } + + // Ignore info messages: the login does not pass them to the message handler. + if (!error.is_message && !pending_login_error->occurred) { + pending_login_error->error = error; + pending_login_error->occurred = 1; + } + + return 1; +} + int tinytds_err_handler(DBPROCESS *dbproc, int severity, int dberr, int oserr, char *dberrstr, char *oserrstr) { static const char *source = "error"; @@ -185,6 +220,10 @@ int tinytds_err_handler(DBPROCESS *dbproc, int severity, int dberr, int oserr, c strncpy(error_data.error, dberrstr, ERROR_MSG_SIZE); strncpy(error_data.source, source, ERROR_MSG_SIZE); + if (defer_login_error(userdata, error_data)) { + return return_value; + } + /* When in non-blocking mode we need to store the exception data to throw it once the blocking call returns, otherwise we will segfault ruby since part @@ -222,6 +261,10 @@ int tinytds_msg_handler(DBPROCESS *dbproc, DBINT msgno, int msgstate, int severi strncpy(error_data.error, msgtext, ERROR_MSG_SIZE); strncpy(error_data.source, source, ERROR_MSG_SIZE); + if (defer_login_error(userdata, error_data)) { + return 0; + } + // See tinytds_err_handler() for info about why we do this if (userdata && userdata->nonblocking) { /* @@ -495,10 +538,25 @@ static VALUE rb_tinytds_connect(VALUE self, VALUE opts) DBSETLUTF16(cwrap->login, 0); } - cwrap->client = dbopen(cwrap->login, StringValueCStr(dataserver)); + char *dataserver_name = StringValueCStr(dataserver); + tinytds_login_error login_error = { .occurred = 0 }; + pending_login_error = &login_error; + cwrap->client = dbopen(cwrap->login, dataserver_name); + pending_login_error = NULL; + + if (login_error.occurred) { + if (cwrap->client) { + dbclose(cwrap->client); + cwrap->client = NULL; + } + + rb_exc_raise(rb_tinytds_new_error(login_error.error)); + } if (cwrap->client) { if (dbtds(cwrap->client) < 11) { + dbclose(cwrap->client); + cwrap->client = NULL; rb_raise(cTinyTdsError, "connecting with a TDS version older than 7.3!"); } diff --git a/test/client_test.rb b/test/client_test.rb index 4662f9c7..029bd998 100644 --- a/test/client_test.rb +++ b/test/client_test.rb @@ -186,6 +186,14 @@ class ClientTest < TinyTds::TestCase assert_new_connections_work end + it "raises TinyTds exception when the negotiated TDS version is older than 7.3" do + action = lambda { new_connection(tds_version: "7.1") } + assert_raise_tinytds_error(action) do |e| + assert_match %r{older than 7.3}i, e.message + end + assert_new_connections_work + end + it "raises TinyTds exception with wrong :username" do skip if ENV["CI"] && sqlserver_azure? # Some issue with db_error_number. options = connection_options username: "willnotwork"