Skip to content
This repository was archived by the owner on Mar 26, 2024. It is now read-only.

Commit 80eb5c9

Browse files
tamirdsodabrew
authored andcommitted
Use Thread.handle_interrupt to protect query
When available, we prevent `Timeout.timeout` from corrupting connections using `Thread.handle_interrupt`. Fixes brianmario#542. Revert "Update specs for Ruby 2.1 Timeout behavior by specifying 'Timeout::Error' as the klass parameter." This reverts commit d0a5199.
1 parent d572951 commit 80eb5c9

File tree

3 files changed

+35
-43
lines changed

3 files changed

+35
-43
lines changed

ext/mysql2/client.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1259,7 +1259,6 @@ void init_mysql2_client() {
12591259
rb_define_singleton_method(cMysql2Client, "info", rb_mysql_client_info, 0);
12601260

12611261
rb_define_method(cMysql2Client, "close", rb_mysql_client_close, 0);
1262-
rb_define_method(cMysql2Client, "query", rb_mysql_client_query, -1);
12631262
rb_define_method(cMysql2Client, "abandon_results!", rb_mysql_client_abandon_results, 0);
12641263
rb_define_method(cMysql2Client, "escape", rb_mysql_client_real_escape, 1);
12651264
rb_define_method(cMysql2Client, "server_info", rb_mysql_client_server_info, 0);
@@ -1280,6 +1279,7 @@ void init_mysql2_client() {
12801279
rb_define_method(cMysql2Client, "encoding", rb_mysql_client_encoding, 0);
12811280
#endif
12821281

1282+
rb_define_private_method(cMysql2Client, "_query", rb_mysql_client_query, -1);
12831283
rb_define_private_method(cMysql2Client, "connect_timeout=", set_connect_timeout, 1);
12841284
rb_define_private_method(cMysql2Client, "read_timeout=", set_read_timeout, 1);
12851285
rb_define_private_method(cMysql2Client, "write_timeout=", set_write_timeout, 1);

lib/mysql2/client.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,17 @@ def self.default_query_options
7474
@@default_query_options
7575
end
7676

77+
if Thread.respond_to?(:handle_interrupt)
78+
def query(*args, &block)
79+
Thread.handle_interrupt(Timeout::ExitException => :never) do
80+
_query(*args, &block)
81+
end
82+
end
83+
else
84+
alias_method :query, :_query
85+
public :query
86+
end
87+
7788
def query_info
7889
info = query_info_string
7990
return {} unless info

spec/mysql2/client_spec.rb

Lines changed: 23 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -462,59 +462,40 @@ def connect *args
462462
}.should raise_error(Mysql2::Error)
463463
end
464464

465-
it "should close the connection when an exception is raised" do
466-
begin
467-
Timeout.timeout(1, Timeout::Error) do
468-
@client.query("SELECT sleep(2)")
469-
end
470-
rescue Timeout::Error
471-
end
472465

473-
lambda {
474-
@client.query("SELECT 1")
475-
}.should raise_error(Mysql2::Error, 'closed MySQL connection')
466+
it 'should be impervious to connection-corrupting timeouts ' do
467+
pending('`Thread.handle_interrupt` is not defined') unless Thread.respond_to?(:handle_interrupt)
468+
# attempt to break the connection
469+
expect { Timeout.timeout(0.1) { @client.query('SELECT SLEEP(1)') } }.to raise_error(Timeout::Error)
470+
471+
# expect the connection to not be broken
472+
expect { @client.query('SELECT 1') }.to_not raise_error
476473
end
477474

478-
it "should handle Timeouts without leaving the connection hanging if reconnect is true" do
479-
client = Mysql2::Client.new(DatabaseCredentials['root'].merge(:reconnect => true))
480-
begin
481-
Timeout.timeout(1, Timeout::Error) do
482-
client.query("SELECT sleep(2)")
483-
end
484-
rescue Timeout::Error
475+
context 'when a non-standard exception class is raised' do
476+
it "should close the connection when an exception is raised" do
477+
expect { Timeout.timeout(0.1, ArgumentError) { @client.query('SELECT SLEEP(1)') } }.to raise_error(ArgumentError)
478+
expect { @client.query('SELECT 1') }.to raise_error(Mysql2::Error, 'closed MySQL connection')
485479
end
486480

487-
lambda {
488-
client.query("SELECT 1")
489-
}.should_not raise_error(Mysql2::Error)
490-
end
481+
it "should handle Timeouts without leaving the connection hanging if reconnect is true" do
482+
client = Mysql2::Client.new(DatabaseCredentials['root'].merge(:reconnect => true))
491483

492-
it "should handle Timeouts without leaving the connection hanging if reconnect is set to true after construction true" do
493-
client = Mysql2::Client.new(DatabaseCredentials['root'])
494-
begin
495-
Timeout.timeout(1, Timeout::Error) do
496-
client.query("SELECT sleep(2)")
497-
end
498-
rescue Timeout::Error
484+
expect { Timeout.timeout(0.1, ArgumentError) { client.query('SELECT SLEEP(1)') } }.to raise_error(ArgumentError)
485+
expect { client.query('SELECT 1') }.to_not raise_error
499486
end
500487

501-
lambda {
502-
client.query("SELECT 1")
503-
}.should raise_error(Mysql2::Error)
488+
it "should handle Timeouts without leaving the connection hanging if reconnect is set to true after construction true" do
489+
client = Mysql2::Client.new(DatabaseCredentials['root'])
504490

505-
client.reconnect = true
491+
expect { Timeout.timeout(0.1, ArgumentError) { client.query('SELECT SLEEP(1)') } }.to raise_error(ArgumentError)
492+
expect { client.query('SELECT 1') }.to raise_error(Mysql2::Error)
506493

507-
begin
508-
Timeout.timeout(1, Timeout::Error) do
509-
client.query("SELECT sleep(2)")
510-
end
511-
rescue Timeout::Error
512-
end
513-
514-
lambda {
515-
client.query("SELECT 1")
516-
}.should_not raise_error(Mysql2::Error)
494+
client.reconnect = true
517495

496+
expect { Timeout.timeout(0.1, ArgumentError) { client.query('SELECT SLEEP(1)') } }.to raise_error(ArgumentError)
497+
expect { client.query('SELECT 1') }.to_not raise_error
498+
end
518499
end
519500

520501
it "threaded queries should be supported" do

0 commit comments

Comments
 (0)