Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use vio_is_connected to check connected state #1022

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,10 @@ Style/TrailingCommaInLiteral:

Style/TrivialAccessors:
AllowPredicates: true

Metrics/BlockLength:
Exclude:
- 'benchmark/setup_db.rb'
- '**/*.rake'
- 'spec/spec_helper.rb'
- 'spec/**/*_spec.rb'
5 changes: 0 additions & 5 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@ Layout/IndentHeredoc:
Metrics/AbcSize:
Max: 90

# Offense count: 31
# Configuration parameters: CountComments, ExcludedMethods.
Metrics/BlockLength:
Max: 860

# Offense count: 1
# Configuration parameters: CountBlocks.
Metrics/BlockNesting:
Expand Down
36 changes: 25 additions & 11 deletions ext/mysql2/client.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

VALUE cMysql2Client;
extern VALUE mMysql2, cMysql2Error, cMysql2TimeoutError;
static VALUE sym_id, sym_version, sym_header_version, sym_async, sym_symbolize_keys, sym_as, sym_array, sym_stream;
static VALUE sym_id, sym_version, sym_header_version, sym_async, sym_symbolize_keys, sym_as, sym_array, sym_stream, sym_has_vio_is_connected;
static VALUE sym_no_good_index_used, sym_no_index_used, sym_query_was_slow;
static ID intern_brackets, intern_merge, intern_merge_bang, intern_new_with_args;

Expand All @@ -25,10 +25,17 @@ static ID intern_brackets, intern_merge, intern_merge_bang, intern_new_with_args
rb_raise(cMysql2Error, "MySQL client is not initialized"); \
}

#if defined(HAVE_VIO_IS_CONNECTED)
my_bool vio_is_connected(Vio *vio);
#define VIO_IS_CONNECTED(wrapper) vio_is_connected(wrapper->client->net.vio)
#else
#define VIO_IS_CONNECTED(wrapper) 1
#endif

#if defined(HAVE_MYSQL_NET_VIO) || defined(HAVE_ST_NET_VIO)
#define CONNECTED(wrapper) (wrapper->client->net.vio != NULL && wrapper->client->net.fd != -1)
#define CONNECTED(wrapper) (wrapper->client->net.vio != NULL && wrapper->client->net.fd != -1 && VIO_IS_CONNECTED(wrapper))
#elif defined(HAVE_MYSQL_NET_PVIO) || defined(HAVE_ST_NET_PVIO)
#define CONNECTED(wrapper) (wrapper->client->net.pvio != NULL && wrapper->client->net.fd != -1)
#define CONNECTED(wrapper) (wrapper->client->net.pvio != NULL && wrapper->client->net.fd != -1 && VIO_IS_CONNECTED(wrapper))
#endif

#define REQUIRE_CONNECTED(wrapper) \
Expand Down Expand Up @@ -946,6 +953,11 @@ static VALUE _mysql_client_options(VALUE self, int opt, VALUE value) {
*/
static VALUE rb_mysql_client_info(RB_MYSQL_UNUSED VALUE klass) {
VALUE version_info, version, header_version;
#if defined(HAVE_VIO_IS_CONNECTED)
VALUE has_vio_is_connected = Qtrue;
#else
VALUE has_vio_is_connected = Qfalse;
#endif
version_info = rb_hash_new();

version = rb_str_new2(mysql_get_client_info());
Expand All @@ -957,6 +969,7 @@ static VALUE rb_mysql_client_info(RB_MYSQL_UNUSED VALUE klass) {
rb_hash_aset(version_info, sym_id, LONG2NUM(mysql_get_client_version()));
rb_hash_aset(version_info, sym_version, version);
rb_hash_aset(version_info, sym_header_version, header_version);
rb_hash_aset(version_info, sym_has_vio_is_connected, has_vio_is_connected);

return version_info;
}
Expand Down Expand Up @@ -1454,14 +1467,15 @@ void init_mysql2_client() {
rb_define_private_method(cMysql2Client, "connect", rb_mysql_connect, 8);
rb_define_private_method(cMysql2Client, "_query", rb_mysql_query, 2);

sym_id = ID2SYM(rb_intern("id"));
sym_version = ID2SYM(rb_intern("version"));
sym_header_version = ID2SYM(rb_intern("header_version"));
sym_async = ID2SYM(rb_intern("async"));
sym_symbolize_keys = ID2SYM(rb_intern("symbolize_keys"));
sym_as = ID2SYM(rb_intern("as"));
sym_array = ID2SYM(rb_intern("array"));
sym_stream = ID2SYM(rb_intern("stream"));
sym_id = ID2SYM(rb_intern("id"));
sym_version = ID2SYM(rb_intern("version"));
sym_header_version = ID2SYM(rb_intern("header_version"));
sym_async = ID2SYM(rb_intern("async"));
sym_symbolize_keys = ID2SYM(rb_intern("symbolize_keys"));
sym_as = ID2SYM(rb_intern("as"));
sym_array = ID2SYM(rb_intern("array"));
sym_stream = ID2SYM(rb_intern("stream"));
sym_has_vio_is_connected = ID2SYM(rb_intern("has_vio_is_connected"));

sym_no_good_index_used = ID2SYM(rb_intern("no_good_index_used"));
sym_no_index_used = ID2SYM(rb_intern("no_index_used"));
Expand Down
1 change: 1 addition & 0 deletions ext/mysql2/extconf.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ def add_ssl_defines(header)
add_ssl_defines(mysql_h)
have_struct_member('MYSQL', 'net.vio', mysql_h)
have_struct_member('MYSQL', 'net.pvio', mysql_h)
have_func('vio_is_connected')

# These constants are actually enums, so they cannot be detected by #ifdef in C code.
have_const('MYSQL_ENABLE_CLEARTEXT_PLUGIN', mysql_h)
Expand Down
16 changes: 15 additions & 1 deletion spec/mysql2/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,20 @@ def run_gc
@client.close
expect(@client.closed?).to eql(true)
end

it "should detect a closed connection" do
skip "libmysqlclient does not export vio_is_connected()" unless Mysql2::Client.info[:has_vio_is_connected]
connection_id = @client.thread_id
Thread.new do
sleep(0.1)
Mysql2::Client.new(DatabaseCredentials['root']).tap do |supervisor|
supervisor.query("KILL #{connection_id}")
end.close
end
expect(@client.query("SELECT 1").first["1"]).to eql(1)
sleep(0.2)
expect(@client.closed?).to be true
end
end

it "should not try to query closed mysql connection" do
Expand Down Expand Up @@ -573,7 +587,7 @@ def run_gc
end
expect do
@client.query("SELECT SLEEP(1)")
end.to raise_error(Mysql2::Error, /Lost connection to MySQL server/)
end.to raise_error(Mysql2::Error)

if RUBY_PLATFORM !~ /mingw|mswin/
expect do
Expand Down