Skip to content

Commit

Permalink
Fix Timeout interrupt handling on Ruby 2.3 and protect Mysql2::Statem…
Browse files Browse the repository at this point in the history
…ent#execute

Timeout::ExitException was removed in Ruby 2.3.0, 2.2.3, and 2.1.8,
in favor of Timeout::Error. Backwards compatible aliases are provided
for Ruby 2.1.x and 2.2.x, but not earlier verions.

With thanks to @jeremy for PR #671 and @yui-knk for PR #677, this commit
also protects prepared statements from being interrupted, so the compat
shim is in Mysql2::Util.
  • Loading branch information
sodabrew committed Sep 14, 2015
1 parent 60b3aa0 commit a4b0489
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 5 deletions.
2 changes: 1 addition & 1 deletion ext/mysql2/statement.c
Expand Up @@ -470,7 +470,7 @@ void init_mysql2_statement() {

rb_define_method(cMysql2Statement, "param_count", param_count, 0);
rb_define_method(cMysql2Statement, "field_count", field_count, 0);
rb_define_method(cMysql2Statement, "execute", execute, -1);
rb_define_method(cMysql2Statement, "_execute", execute, -1);
rb_define_method(cMysql2Statement, "fields", fields, 0);
rb_define_method(cMysql2Statement, "last_id", rb_mysql_stmt_last_id, 0);
rb_define_method(cMysql2Statement, "affected_rows", rb_mysql_stmt_affected_rows, 0);
Expand Down
18 changes: 18 additions & 0 deletions lib/mysql2.rb
Expand Up @@ -62,5 +62,23 @@ def self.key_hash_as_symbols(hash)
return nil unless hash
Hash[hash.map { |k, v| [k.to_sym, v] }]
end

#
# In Mysql2::Client#query and Mysql2::Statement#execute,
# Thread#handle_interrupt is used to prevent Timeout#timeout
# from interrupting query execution.
#
# Timeout::ExitException was removed in Ruby 2.3.0, 2.2.3, and 2.1.8,
# but is present in earlier 2.1.x and 2.2.x, so we provide a shim.
#
if Thread.respond_to?(:handle_interrupt)
require 'timeout'
# rubocop:disable Style/ConstantName
TimeoutError = if defined?(::Timeout::ExitException)
::Timeout::ExitException
else
::Timeout::Error
end
end
end
end
4 changes: 1 addition & 3 deletions lib/mysql2/client.rb
Expand Up @@ -80,10 +80,8 @@ def initialize(opts = {})
end

if Thread.respond_to?(:handle_interrupt)
require 'timeout'

def query(sql, options = {})
Thread.handle_interrupt(::Timeout::ExitException => :never) do
Thread.handle_interrupt(::Mysql2::Util::TimeoutError => :never) do
_query(sql, @query_options.merge(options))
end
end
Expand Down
12 changes: 12 additions & 0 deletions lib/mysql2/statement.rb
@@ -1,5 +1,17 @@
module Mysql2
class Statement
include Enumerable

if Thread.respond_to?(:handle_interrupt)
def execute(*args)
Thread.handle_interrupt(::Mysql2::Util::TimeoutError => :never) do
_execute(*args)
end
end
else
def execute(*args)
_execute(*args)
end
end
end
end
16 changes: 15 additions & 1 deletion spec/mysql2/client_spec.rb
Expand Up @@ -467,7 +467,7 @@ def run_gc
}.to raise_error(Mysql2::Error)
end

it 'should be impervious to connection-corrupting timeouts ' do
it 'should be impervious to connection-corrupting timeouts in #query' do
pending('`Thread.handle_interrupt` is not defined') unless Thread.respond_to?(:handle_interrupt)
# attempt to break the connection
expect { Timeout.timeout(0.1) { @client.query('SELECT SLEEP(0.2)') } }.to raise_error(Timeout::Error)
Expand All @@ -476,6 +476,20 @@ def run_gc
expect { @client.query('SELECT 1') }.to_not raise_error
end

it 'should be impervious to connection-corrupting timeouts in #execute' do
# the statement handle gets corrupted and will segfault the tests if interrupted,
# so we can't even use pending on this test, really have to skip it on older Rubies.
skip('`Thread.handle_interrupt` is not defined') unless Thread.respond_to?(:handle_interrupt)

# attempt to break the connection
stmt = @client.prepare('SELECT SLEEP(?)')
expect { Timeout.timeout(0.1) { stmt.execute(0.2) } }.to raise_error(Timeout::Error)
stmt.close

# expect the connection to not be broken
expect { @client.query('SELECT 1') }.to_not raise_error
end

context 'when a non-standard exception class is raised' do
it "should close the connection when an exception is raised" do
expect { Timeout.timeout(0.1, ArgumentError) { @client.query('SELECT SLEEP(1)') } }.to raise_error(ArgumentError)
Expand Down

0 comments on commit a4b0489

Please sign in to comment.