From 8bfbfa2708abfc420827eaef0e817657d9ba9b9d Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 18 Aug 2010 19:06:05 -0700 Subject: [PATCH] fix signal handling when waiting on queries This reverts the single-threaded select() optimization from commits 9a63a587c01e4ac116ba79e9cc23a3b3fddf589d and 0190457dbd6d963de1a16a3c5165ad346a0571d8. Under Ruby 1.9, the reverted optimization caused signal handlers to be delayed until the socket became readable. Under Ruby 1.8, matters are worse as receiving a signal during select() causes Errno::EINTR to be raised. There is now a (somewhat fragile) spec for testing signal handling during a "SELECT sleep(2)" query. Furthermore, the performance difference I measured on benchmark/thread_alone.rb was negligible (over 1000 iterations) between the two: Ruby 1.8.7-p249 Rehearsal ---------------------------------------------------- select 0.040000 0.020000 0.060000 ( 0.119448) rb_thread_select 0.050000 0.020000 0.070000 ( 0.132091) ------------------------------------------- total: 0.130000sec user system total real select 0.030000 0.030000 0.060000 ( 0.116471) rb_thread_select 0.050000 0.020000 0.070000 ( 0.119874) Ruby 1.9.2-p0 Rehearsal ---------------------------------------------------- select 0.050000 0.030000 0.080000 ( 0.134208) rb_thread_select 0.080000 0.000000 0.080000 ( 0.141316) ------------------------------------------- total: 0.160000sec user system total real select 0.050000 0.020000 0.070000 ( 0.123325) rb_thread_select 0.060000 0.010000 0.070000 ( 0.124075) Benchmarks were performed on an _empty_ mysql2_test table to maximize the relative time for the select(2)-related code path (vs reading data). The test was run on Debian Lenny, x86_64 and a stock 2.6.35.2 Linux kernel. Hardware was a Core2 Duo @ 1.6GHz with the performance CPU governor while on AC power to eliminate CPU speed fluctuations during the test. --- ext/mysql2/client.c | 4 +--- spec/mysql2/client_spec.rb | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/ext/mysql2/client.c b/ext/mysql2/client.c index 2650b2f..f2942c2 100644 --- a/ext/mysql2/client.c +++ b/ext/mysql2/client.c @@ -257,7 +257,6 @@ static VALUE rb_mysql_client_query(int argc, VALUE * argv, VALUE self) { int fd, retval; int async = 0; VALUE opts, defaults; - int(*selector)(int, fd_set *, fd_set *, fd_set *, struct timeval *) = NULL; GET_CLIENT(self) REQUIRE_OPEN_DB(client); @@ -299,12 +298,11 @@ static VALUE rb_mysql_client_query(int argc, VALUE * argv, VALUE self) { // the below code is largely from do_mysql // http://github.com/datamapper/do fd = client->net.fd; - selector = rb_thread_alone() ? *select : *rb_thread_select; for(;;) { FD_ZERO(&fdset); FD_SET(fd, &fdset); - retval = selector(fd + 1, &fdset, NULL, NULL, NULL); + retval = rb_thread_select(fd + 1, &fdset, NULL, NULL, NULL); if (retval < 0) { rb_sys_fail(0); diff --git a/spec/mysql2/client_spec.rb b/spec/mysql2/client_spec.rb index 7c5c8ce..2df7a77 100644 --- a/spec/mysql2/client_spec.rb +++ b/spec/mysql2/client_spec.rb @@ -78,6 +78,33 @@ describe Mysql2::Client do @client.query("SELECT 1") }.should raise_error(Mysql2::Error) end + + # XXX this test is not deterministic (because Unix signal handling is not) + # and may fail on a loaded system + it "should run signal handlers while waiting for a response" do + mark = {} + trap(:USR1) { mark[:USR1] = Time.now } + begin + mark[:START] = Time.now + pid = fork do + sleep 1 # wait for client "SELECT sleep(2)" query to start + Process.kill(:USR1, Process.ppid) + sleep # wait for explicit kill to prevent GC disconnect + end + @client.query("SELECT sleep(2)") + mark[:END] = Time.now + mark.include?(:USR1).should be_true + (mark[:USR1] - mark[:START]).should >= 1 + (mark[:USR1] - mark[:START]).should < 1.1 + (mark[:END] - mark[:USR1]).should > 0.9 + (mark[:END] - mark[:START]).should >= 2 + (mark[:END] - mark[:START]).should < 2.1 + Process.kill(:TERM, pid) + Process.waitpid2(pid) + ensure + trap(:USR1, 'DEFAULT') + end + end if RUBY_PLATFORM !~ /mingw|mswin/ end it "should respond to #escape" do