From 3c498dd4a2d915d8ceacecfba8285d42b0f0938f Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 11 Dec 2023 15:21:40 +0100 Subject: [PATCH] Call `getaddrinfo` from Ruby `getaddrinfo` is notorious for not having a timeout, which can make it hang for a very long time. Since Trilogy releases the GVL before calling it, that can cause the whole VM to be unresponsive and no longer respond to ctrl+c etc. In 3.3.0, Ruby is now doing name resolution from a background thread, making `Socket.getaddrinfo` interruptible. So by doing the name resolution on the Ruby side, we should avoid such scenarios. --- contrib/ruby/ext/trilogy-ruby/cext.c | 8 +++++++- contrib/ruby/lib/trilogy.rb | 13 +++++++++++++ contrib/ruby/test/client_test.rb | 6 ++++-- inc/trilogy/socket.h | 1 + src/socket.c | 5 ++++- 5 files changed, 29 insertions(+), 4 deletions(-) diff --git a/contrib/ruby/ext/trilogy-ruby/cext.c b/contrib/ruby/ext/trilogy-ruby/cext.c index 69f84511..3969f890 100644 --- a/contrib/ruby/ext/trilogy-ruby/cext.c +++ b/contrib/ruby/ext/trilogy-ruby/cext.c @@ -25,7 +25,7 @@ static ID id_socket, id_host, id_port, id_username, id_password, id_found_rows, id_ivar_affected_rows, id_ivar_fields, id_ivar_last_insert_id, id_ivar_rows, id_ivar_query_time, id_password, id_database, id_enable_cleartext_plugin, id_ssl_ca, id_ssl_capath, id_ssl_cert, id_ssl_cipher, id_ssl_crl, id_ssl_crlpath, id_ssl_key, id_ssl_mode, id_tls_ciphersuites, id_tls_min_version, id_tls_max_version, id_multi_statement, id_multi_result, - id_from_code, id_from_errno, id_connection_options, id_max_allowed_packet; + id_from_code, id_from_errno, id_connection_options, id_max_allowed_packet, id_ip_address; struct trilogy_ctx { trilogy_conn_t conn; @@ -509,6 +509,11 @@ static VALUE rb_trilogy_connect(VALUE self, VALUE encoding, VALUE charset, VALUE Check_Type(val, T_FIXNUM); connopt.port = NUM2USHORT(val); } + + if ((val = rb_hash_lookup(opts, ID2SYM(id_ip_address))) != Qnil) { + Check_Type(val, T_STRING); + connopt.ip_address = StringValueCStr(val); + } } else { connopt.path = (char *)"/tmp/mysql.sock"; @@ -1215,6 +1220,7 @@ RUBY_FUNC_EXPORTED void Init_cext(void) id_socket = rb_intern("socket"); id_host = rb_intern("host"); + id_ip_address = rb_intern("ip_address"); id_port = rb_intern("port"); id_username = rb_intern("username"); id_password = rb_intern("password"); diff --git a/contrib/ruby/lib/trilogy.rb b/contrib/ruby/lib/trilogy.rb index bde8a033..0156a105 100644 --- a/contrib/ruby/lib/trilogy.rb +++ b/contrib/ruby/lib/trilogy.rb @@ -8,10 +8,23 @@ class Trilogy def initialize(options = {}) + options = options.dup options[:port] = options[:port].to_i if options[:port] mysql_encoding = options[:encoding] || "utf8mb4" encoding = Trilogy::Encoding.find(mysql_encoding) charset = Trilogy::Encoding.charset(mysql_encoding) + + if options[:host] + addrs = begin + Socket.getaddrinfo(options[:host], options[:port], :PF_UNSPEC, :SOCK_STREAM) + rescue SocketError + raise Trilogy::BaseConnectionError, "Couldn't resolve host: #{options[:host].inspect}" + end + + addrs.sort_by!(&:first) # Priority to IPv4 + options[:ip_address] = addrs.first[3] + end + @connection_options = options @connected_host = nil diff --git a/contrib/ruby/test/client_test.rb b/contrib/ruby/test/client_test.rb index f9aac0e7..f9541a48 100644 --- a/contrib/ruby/test/client_test.rb +++ b/contrib/ruby/test/client_test.rb @@ -63,7 +63,9 @@ def test_trilogy_connection_options ssl_mode: 4, tls_min_version: 3, } - assert_equal expected_connection_options, client.connection_options + actual_options = client.connection_options.dup + actual_options.delete(:ip_address) + assert_equal expected_connection_options, actual_options end def test_trilogy_ping @@ -968,7 +970,7 @@ def test_connection_invalid_dns ex = assert_raises Trilogy::ConnectionError do new_tcp_client(host: "mysql.invalid", port: 3306) end - assert_equal "trilogy_connect - unable to connect to mysql.invalid:3306: TRILOGY_DNS_ERROR", ex.message + assert_includes ex.message, %{Couldn't resolve host: "mysql.invalid"} end def test_memsize diff --git a/inc/trilogy/socket.h b/inc/trilogy/socket.h index 6de19476..1ed66bff 100644 --- a/inc/trilogy/socket.h +++ b/inc/trilogy/socket.h @@ -37,6 +37,7 @@ typedef enum { typedef struct { char *hostname; + char *ip_address; char *path; char *database; char *username; diff --git a/src/socket.c b/src/socket.c index 46b2aa92..35b3205e 100644 --- a/src/socket.c +++ b/src/socket.c @@ -113,6 +113,7 @@ static int _cb_raw_close(trilogy_sock_t *_sock) } free(sock->base.opts.hostname); + free(sock->base.opts.ip_address); free(sock->base.opts.path); free(sock->base.opts.database); free(sock->base.opts.username); @@ -317,6 +318,7 @@ trilogy_sock_t *trilogy_sock_new(const trilogy_sockopt_t *opts) sock->base.opts = *opts; sock->base.opts.hostname = strdupnullok(opts->hostname); + sock->base.opts.ip_address = strdupnullok(opts->ip_address); sock->base.opts.path = strdupnullok(opts->path); sock->base.opts.database = strdupnullok(opts->database); sock->base.opts.username = strdupnullok(opts->username); @@ -352,7 +354,8 @@ int trilogy_sock_resolve(trilogy_sock_t *_sock) char port[6]; snprintf(port, sizeof(port), "%hu", sock->base.opts.port); - if (getaddrinfo(sock->base.opts.hostname, port, &hint, &sock->addr) != 0) { + char *address = sock->base.opts.ip_address ? sock->base.opts.ip_address : sock->base.opts.hostname; + if (getaddrinfo(address, port, &hint, &sock->addr) != 0) { return TRILOGY_DNS_ERR; } } else if (sock->base.opts.path != NULL) {