From 52a94a7d2c3f0a50fea5e7c0aaa77a854678299c Mon Sep 17 00:00:00 2001 From: John Rhoads Date: Wed, 16 Sep 2026 11:43:27 -0600 Subject: [PATCH] Add Client#ping(timeout:) with FreeTDS-bounded liveness check Temporarily lower DBSETTIME for SELECT 1 so dead/idle-RST handles fail in seconds instead of the connection's long query timeout; restore on success. Returns false on failure so callers can discard the client. Addresses the liveness-probe discussion in rails-sqlserver/activerecord-sqlserver-adapter#1396 without changing passive active?/dead? semantics. Co-authored-by: Cursor --- CHANGELOG.md | 2 ++ ext/tiny_tds/client.c | 55 ++++++++++++++++++++++++++++++++++++++---- ext/tiny_tds/client.h | 2 ++ lib/tiny_tds/client.rb | 22 +++++++++++++++++ test/client_test.rb | 47 ++++++++++++++++++++++++++++++++++++ 5 files changed, 123 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 373e3695..f73209cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## (unreleased) +* Add `Client#ping(timeout:)` for a short FreeTDS-bounded liveness round-trip, plus `query_timeout` / `query_timeout=` to change `DBSETTIME` on a live client. + ## 3.4.0 * Add Ruby 4.0 to the cross compile list diff --git a/ext/tiny_tds/client.c b/ext/tiny_tds/client.c index 5f98dee4..cc9ceb39 100644 --- a/ext/tiny_tds/client.c +++ b/ext/tiny_tds/client.c @@ -295,10 +295,31 @@ static VALUE allocate(VALUE klass) cwrap->charset = Qnil; cwrap->userdata = malloc(sizeof(tinytds_client_userdata)); cwrap->userdata->closed = 1; + cwrap->query_timeout = 0; rb_tinytds_client_reset_userdata(cwrap->userdata); return obj; } +static int tinytds_apply_query_timeout(tinytds_client_wrapper *cwrap, int seconds) +{ + VALUE timeout_string; + + if (seconds < 0) { + return 0; + } + + timeout_string = rb_sprintf("%d", seconds); + + if (dbsetopt(cwrap->client, DBSETTIME, StringValueCStr(timeout_string), 0) == FAIL) { + if (dbsettime(seconds) == FAIL) { + return 0; + } + } + + cwrap->query_timeout = seconds; + return 1; +} + // TinyTds::Client (public) @@ -346,6 +367,30 @@ static VALUE rb_tinytds_sqlsent(VALUE self) return cwrap->userdata->dbsql_sent ? Qtrue : Qfalse; } +static VALUE rb_tinytds_query_timeout(VALUE self) +{ + GET_CLIENT_WRAPPER(self); + return INT2NUM(cwrap->query_timeout); +} + +static VALUE rb_tinytds_query_timeout_set(VALUE self, VALUE value) +{ + int seconds; + GET_CLIENT_WRAPPER(self); + REQUIRE_OPEN_CLIENT(cwrap); + seconds = NUM2INT(value); + + if (seconds < 0) { + rb_raise(rb_eArgError, "query timeout must be >= 0"); + } + + if (!tinytds_apply_query_timeout(cwrap, seconds)) { + rb_raise(cTinyTdsError, "failed to set query timeout"); + } + + return INT2NUM(cwrap->query_timeout); +} + static VALUE rb_tinytds_execute(VALUE self, VALUE sql) { VALUE result; @@ -502,7 +547,7 @@ static VALUE rb_tinytds_connect(VALUE self, VALUE opts) rb_raise(cTinyTdsError, "connecting with a TDS version older than 7.3!"); } - VALUE transposed_encoding, timeout_string; + VALUE transposed_encoding; cwrap->closed = 0; cwrap->charset = charset; @@ -512,10 +557,8 @@ static VALUE rb_tinytds_connect(VALUE self, VALUE opts) } if (!NIL_P(timeout)) { - timeout_string = rb_sprintf("%"PRIsVALUE"", timeout); - - if (dbsetopt(cwrap->client, DBSETTIME, StringValueCStr(timeout_string), 0) == FAIL) { - dbsettime(NUM2INT(timeout)); + if (!tinytds_apply_query_timeout(cwrap, NUM2INT(timeout))) { + rb_raise(cTinyTdsError, "failed to set query timeout"); } } @@ -550,6 +593,8 @@ void init_tinytds_client() rb_define_method(cTinyTdsClient, "dead?", rb_tinytds_dead, 0); rb_define_method(cTinyTdsClient, "sqlsent?", rb_tinytds_sqlsent, 0); rb_define_method(cTinyTdsClient, "execute", rb_tinytds_execute, 1); + rb_define_method(cTinyTdsClient, "query_timeout", rb_tinytds_query_timeout, 0); + rb_define_method(cTinyTdsClient, "query_timeout=", rb_tinytds_query_timeout_set, 1); rb_define_method(cTinyTdsClient, "charset", rb_tinytds_charset, 0); rb_define_method(cTinyTdsClient, "encoding", rb_tinytds_encoding, 0); rb_define_method(cTinyTdsClient, "escape", rb_tinytds_escape, 1); diff --git a/ext/tiny_tds/client.h b/ext/tiny_tds/client.h index dcd6087c..e01f9b0b 100644 --- a/ext/tiny_tds/client.h +++ b/ext/tiny_tds/client.h @@ -40,6 +40,8 @@ typedef struct { tinytds_client_userdata *userdata; const char *identity_insert_sql; rb_encoding *encoding; + /* Last FreeTDS DBSETTIME value applied to this client (seconds). */ + int query_timeout; } tinytds_client_wrapper; VALUE rb_tinytds_raise_error(DBPROCESS *dbproc, tinytds_errordata error); diff --git a/lib/tiny_tds/client.rb b/lib/tiny_tds/client.rb index 92a195b7..15666af2 100644 --- a/lib/tiny_tds/client.rb +++ b/lib/tiny_tds/client.rb @@ -70,6 +70,28 @@ def active? !closed? && !dead? end + # Liveness round-trip with a temporary FreeTDS query timeout (seconds). + # Returns true when SELECT 1 succeeds and the previous timeout is restored. + # Returns false on timeout / dead handle / query failure (caller should discard). + # Raises on a closed client or invalid timeout. + def ping(timeout: 2) + raise TinyTds::Error, "closed connection" if closed? + return false unless active? + + seconds = Integer(timeout) + raise ArgumentError, "timeout must be positive" unless seconds.positive? + + previous = query_timeout + begin + self.query_timeout = seconds + execute("SELECT 1").each { break } + self.query_timeout = previous + true + rescue TinyTds::Error + false + end + end + private def parse_username(opts) diff --git a/test/client_test.rb b/test/client_test.rb index 4662f9c7..7baa96e8 100644 --- a/test/client_test.rb +++ b/test/client_test.rb @@ -6,6 +6,53 @@ class ClientTest < TinyTds::TestCase @client = new_connection end + it "exposes query_timeout from the connect :timeout option" do + assert_equal connection_timeout, @client.query_timeout + end + + it "applies query_timeout= via FreeTDS for subsequent batches" do + @client.query_timeout = 1 + assert_equal 1, @client.query_timeout + action = lambda { @client.execute("WaitFor Delay '00:00:02'").do } + assert_raise_tinytds_error(action) do |e| + assert_equal 20003, e.db_error_number + assert_match %r{timed out}i, e.message, "ignore if non-english test run" + end + end + + it "ping returns true and restores the previous query timeout" do + @client.query_timeout = connection_timeout + assert_equal true, @client.ping(timeout: 2) + assert_equal connection_timeout, @client.query_timeout + assert_client_works(@client) + end + + it "ping returns false when the round-trip exceeds the ping timeout" do + client = new_connection timeout: 30 + # Swap the ping batch for a delay longer than the ping timeout so FreeTDS + # SYBETIME fires under the temporary DBSETTIME, not Ruby Timeout. + def client.execute(sql) + if sql == "SELECT 1" + super("WaitFor Delay '00:00:03'") + else + super + end + end + assert_equal false, client.ping(timeout: 1) + ensure + close_client(client) + end + + it "ping raises ArgumentError for non-positive timeout" do + assert_raises(ArgumentError) { @client.ping(timeout: 0) } + assert_raises(ArgumentError) { @client.ping(timeout: -1) } + end + + it "ping raises when the client is closed" do + @client.close + assert_raises(TinyTds::Error) { @client.ping(timeout: 2) } + end + it "must not be closed" do assert !@client.closed? assert @client.active?