Stop using dbcancel as the nogvl unblock function

[emailrhoads]

Summary

• rb_thread_call_without_gvl currently uses dbcancel as the unblock function. MRI invokes that UBF whenever the waiting thread has a pending interrupt, including process-directed signals such as SIGCHLD delivered to main after another thread spawns a child.
• dbcancel then aborts the in-flight batch. Result#each treats FAIL as an empty success, so long queries can return [] even though SQL Server produced rows.
• This keeps releasing the GVL (NULL, NULL) so other Ruby threads can run, but no longer cancels SQL on those interrupts. Client :timeout is unchanged (dbsetinterrupt).
• Tradeoff: Timeout.timeout / Thread#kill wait until the batch finishes or :timeout fires.

Test plan

• New test: another thread reaps child processes during WAITFOR DELAY; the following SELECT 42 must still return a row.
• Existing timeout tests still pass (:timeout via dbsetinterrupt).
• CI on this PR.

[emailrhoads]

@andyundso any thoughts on this idea?

[andyundso]

@emailrhoads I am trying to grasp the consequence of this change. If you have some time and maybe dev environment running, can you add the following test to thread_test.rb and see what happens?

client = new_connection
assert_client_works(client)

thread = Thread.new do
  client.do("waitfor delay '00:00:05'")
end

sleep 0.1

time = Benchmark.measure do
  thread.kill
end

assert time.real < 5

Otherwise I might have some time on the weekend to see myself. Nulling out the unblock function would mean the call just continue to run indefinitely, but maybe I am missing something.

[emailrhoads]

@emailrhoads I am trying to grasp the consequence of this change. If you have some time and maybe dev environment running, can you add the following test to thread_test.rb and see what happens?

client = new_connection
assert_client_works(client)

thread = Thread.new do
  client.do("waitfor delay '00:00:05'")
end

sleep 0.1

time = Benchmark.measure do
  thread.kill
end

assert time.real < 5

Otherwise I might have some time on the weekend to see myself. Nulling out the unblock function would mean the call just continue to run indefinitely, but maybe I am missing something.

@andyundso added the test and a bit more which I think was the spirit of your request at https://github.com/rails-sqlserver/tiny_tds/pull/607/changes#diff-fe76b289030e9af8010639512c4d9fab1f5cfd43207ae9137850ae1949dafae1R82

In short, we lose the ability to do Thread#kill and Timeout.timeout with this change and have it behave the way we would like.
Thread.kill will return quickly, but at Thread.join we are forced to wait for completion anyway.

Here are the actual test results though

Branch	Thread#kill	join after kill
master baseline (#610)	~0.0s	~0.01s (cancelled)
NULL UBF (#607)	~0.0s	~4.9s (WAITFOR ran)

The caller would need to use something like client.cancel to break the running connection instead.

[andyundso]

I am sure I miss something in this discussion.

In short, we lose the ability to do Thread#kill and Timeout.timeout with this change and have it behave the way we would like.

I am not sure if we agree here what the correct behaviour is. Sure, that other threads have influence of the current thread is not ideal, but not being able to kill a running program is also not good.

Thread.kill will return quickly, but at Thread.join we are forced to wait for completion anyway.

Your test table clearly shows that with the current master build, we won't have to wait long at Thread.join.

---

I checked some other DB libraries and the pg gem also does not cancel the query when releasing the GVL. I wanted to double-check the behaviour there.

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'
  gem 'benchmark'
  gem 'pg'
end


time = Benchmark.measure do
  thread = Thread.new do
    conn = PG.connect(
      host: "127.0.0.1",
      port: 5432,
      dbname: "postgres",
      user: "postgres",
      password: "mysecretpassword"
    )
    conn.exec("SELECT pg_sleep(5)")
    conn.close
  end

  sleep(0.5)

  thread.kill
  thread.join
end

puts time.inspect

yields:

#<Benchmark::Tms:0x000070c8fb4d64a0 @label="", @real=0.5003859230000671, @cstime=0.0, @cutime=0.0, @stime=0.00017199999999999854, @utime=0.004820000000000005, @total=0.004992000000000003>

so in short, there might be a way to have both, no thread cancels and still fast runtime.