diff --git a/CHANGELOG.md b/CHANGELOG.md index 373e3695..b59438f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## (unreleased) +* Do not use `dbcancel` as the `rb_thread_call_without_gvl` unblock function. An interrupt (for example SIGCHLD) during `dbsqlok`/`dbresults`/`dbnextrow` aborted the batch and returned empty results. Client `:timeout` is unchanged. + ## 3.4.0 * Add Ruby 4.0 to the cross compile list diff --git a/ext/tiny_tds/result.c b/ext/tiny_tds/result.c index 449f01c7..e77017aa 100644 --- a/ext/tiny_tds/result.c +++ b/ext/tiny_tds/result.c @@ -89,19 +89,17 @@ VALUE rb_tinytds_new_result_obj(tinytds_client_wrapper *cwrap) // No GVL Helpers +/* Do not pass dbcancel as the unblock function. MRI invokes the UBF + whenever the waiting thread has a pending interrupt, including + process-directed signals such as SIGCHLD delivered to main. + dbcancel then aborts the SQL batch and Result#each treats FAIL as + an empty success. Client :timeout still uses dbsetinterrupt. */ #define NOGVL_DBCALL(_dbfunction, _client) ( \ (RETCODE)(intptr_t)rb_thread_call_without_gvl( \ (void *(*)(void *))_dbfunction, _client, \ - (rb_unblock_function_t*)dbcancel_ubf, _client ) \ + NULL, NULL ) \ ) -static void dbcancel_ubf(DBPROCESS *client) -{ - GET_CLIENT_USERDATA(client); - dbcancel(client); - userdata->dbcancel_sent = 1; -} - static void nogvl_setup(DBPROCESS *client) { GET_CLIENT_USERDATA(client); diff --git a/test/client_test.rb b/test/client_test.rb index 4662f9c7..e92919ba 100644 --- a/test/client_test.rb +++ b/test/client_test.rb @@ -1,4 +1,5 @@ require "test_helper" +require "rbconfig" class ClientTest < TinyTds::TestCase describe "with valid credentials" do @@ -95,6 +96,23 @@ class ClientTest < TinyTds::TestCase assert_new_connections_work end + it "does not cancel a query when another thread reaps a child process" do + skip if sqlserver_azure? + client = new_connection timeout: 15 + reaper = Thread.new do + sleep 0.5 + 5.times do + Process.wait(Process.spawn(RbConfig.ruby, "-e", "nil")) + sleep 0.2 + end + end + rows = client.execute("WaitFor Delay '00:00:02'; SELECT 42 AS [n]").each + reaper.join + assert_equal 1, rows.length + assert_equal 42, rows.first["n"] + close_client(client) + end + it "raises TinyTds exception with long query past :timeout option" do client = new_connection timeout: 1 action = lambda { client.execute("WaitFor Delay '00:00:02'").do }