From c18df6d87cb1a26f596218beb8099c709e30aaf4 Mon Sep 17 00:00:00 2001 From: normal Date: Sun, 12 Apr 2015 01:41:51 +0000 Subject: connect_nonblock supports "exception: false" This is for consistency with accept_nonblock arguments and gives a minor speedup from avoiding exceptions. [ruby-core:68838] [Feature #11024] * ext/openssl/ossl_ssl.c (ossl_ssl_connect_nonblock): support `exception: false' * (get_no_exception): move function location * ext/socket/socket.c (sock_connect_nonblock): support `exception: false' * test/openssl/test_pair.rb (test_connect_accept_nonblock_no_exception): test `exception: false' on connect, rename from `test_accept_nonblock_no_exception' * test/socket/test_nonblock.rb (test_connect_nonblock_no_exception): new test Benchmark results: default 0.050000 0.100000 0.150000 ( 0.151307) exception: false 0.030000 0.080000 0.110000 ( 0.108840) ----------------------------8<----------------------- require 'socket' require 'benchmark' require 'io/wait' require 'tmpdir' host = '127.0.0.1' serv = TCPServer.new(host, 0) # UNIX sockets may not hit EINPROGRESS nr = 5000 # few iterations to avoid running out of ports addr = serv.getsockname pid = fork do begin serv.accept.close rescue => e warn "#$$: #{e.message} (#{e.class})" end while true end at_exit { Process.kill(:TERM, pid) } serv.close Benchmark.bmbm do |x| x.report("default") do nr.times do s = Socket.new(:INET, :STREAM) s.setsockopt(:SOL_SOCKET, :SO_REUSEADDR, 1) begin s.connect_nonblock(addr) rescue IO::WaitWritable s.wait_writable end s.close end end x.report("exception: false") do nr.times do s = Socket.new(:INET, :STREAM) s.setsockopt(:SOL_SOCKET, :SO_REUSEADDR, 1) case s.connect_nonblock(addr, exception: false) when :wait_writable s.wait_writable end s.close end end end git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@50254 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 13 +++++++++++++ NEWS | 9 +++++---- ext/openssl/ossl_ssl.c | 34 ++++++++++++++++++++++------------ ext/socket/socket.c | 26 ++++++++++++++++++++++---- test/openssl/test_pair.rb | 24 ++++++++++++++++++++++-- test/socket/test_nonblock.rb | 23 +++++++++++++++++++++++ 6 files changed, 107 insertions(+), 22 deletions(-) diff --git a/ChangeLog b/ChangeLog index bbcea79628..6813769938 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +Sun Apr 12 10:29:14 2015 Eric Wong + + * ext/openssl/ossl_ssl.c (ossl_ssl_connect_nonblock): + support `exception: false' + * (get_no_exception): move function location + * ext/socket/socket.c (sock_connect_nonblock): + support `exception: false' + * test/openssl/test_pair.rb (test_connect_accept_nonblock_no_exception): + test `exception: false' on connect, + rename from `test_accept_nonblock_no_exception' + * test/socket/test_nonblock.rb (test_connect_nonblock_no_exception): + new test + Sun Apr 12 09:57:16 2015 Tanaka Akira * test/ruby/test_file_exhaustive.rb: Test a block device on GNU/Linux. diff --git a/NEWS b/NEWS index a7ab711dd1..da678af22c 100644 --- a/NEWS +++ b/NEWS @@ -36,15 +36,16 @@ with all sufficient information, see the ChangeLog file. === Stdlib updates (outstanding ones only) * Socket - * Socket#accept_nonblock supports `exception :false` to return symbols - Ditto for TCPServer#accept_nonblock, UNIXServer#accept_nonblock + * Socket#accept_nonblock and Socket#connect_nonblock supports + `exception :false` to return symbols. + Ditto for TCPServer#accept_nonblock, UNIXServer#accept_nonblock. * ObjectSpace (objspace) * ObjectSpace.count_imemo_objects is added. * OpenSSL - * OpenSSL::SSL::SSLSocket#accept_nonblock supports `exception: false` - to return symbols + * OpenSSL::SSL::SSLSocket#accept_nonblock and + OpenSSL::SSL::SSLSocket#connect_nonblock supports `exception: false`. === Stdlib compatibility issues (excluding feature bug fixes) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index 1a67d35d11..2e128356aa 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -1330,9 +1330,17 @@ ossl_ssl_connect(VALUE self) return ossl_start_ssl(self, SSL_connect, "SSL_connect", 0, 0); } +static int +get_no_exception(VALUE opts) +{ + if (!NIL_P(opts) && Qfalse == rb_hash_lookup2(opts, sym_exception, Qundef)) + return 1; + return 0; +} + /* * call-seq: - * ssl.connect_nonblock => self + * ssl.connect_nonblock([options]) => self * * Initiates the SSL/TLS handshake as a client in non-blocking manner. * @@ -1347,12 +1355,22 @@ ossl_ssl_connect(VALUE self) * retry * end * + * By specifying `exception: false`, the options hash allows you to indicate + * that connect_nonblock should not raise an IO::WaitReadable or + * IO::WaitWritable exception, but return the symbol :wait_readable or + * :wait_writable instead. */ static VALUE -ossl_ssl_connect_nonblock(VALUE self) +ossl_ssl_connect_nonblock(int argc, VALUE *argv, VALUE self) { + int no_exception; + VALUE opts = Qnil; + + rb_scan_args(argc, argv, "0:", &opts); + no_exception = get_no_exception(opts); + ossl_ssl_setup(self); - return ossl_start_ssl(self, SSL_connect, "SSL_connect", 1, 0); + return ossl_start_ssl(self, SSL_connect, "SSL_connect", 1, no_exception); } /* @@ -1369,14 +1387,6 @@ ossl_ssl_accept(VALUE self) return ossl_start_ssl(self, SSL_accept, "SSL_accept", 0, 0); } -static int -get_no_exception(VALUE opts) -{ - if (!NIL_P(opts) && Qfalse == rb_hash_lookup2(opts, sym_exception, Qundef)) - return 1; - return 0; -} - /* * call-seq: * ssl.accept_nonblock([options]) => self @@ -2235,7 +2245,7 @@ Init_ossl_ssl(void) rb_define_alias(cSSLSocket, "to_io", "io"); rb_define_method(cSSLSocket, "initialize", ossl_ssl_initialize, -1); rb_define_method(cSSLSocket, "connect", ossl_ssl_connect, 0); - rb_define_method(cSSLSocket, "connect_nonblock", ossl_ssl_connect_nonblock, 0); + rb_define_method(cSSLSocket, "connect_nonblock", ossl_ssl_connect_nonblock, -1); rb_define_method(cSSLSocket, "accept", ossl_ssl_accept, 0); rb_define_method(cSSLSocket, "accept_nonblock", ossl_ssl_accept_nonblock, -1); rb_define_method(cSSLSocket, "sysread", ossl_ssl_read, -1); diff --git a/ext/socket/socket.c b/ext/socket/socket.c index bb18409489..a514b9ab09 100644 --- a/ext/socket/socket.c +++ b/ext/socket/socket.c @@ -10,6 +10,8 @@ #include "rubysocket.h" +static VALUE sym_exception, sym_wait_writable; + static VALUE sock_s_unpack_sockaddr_in(VALUE, VALUE); void @@ -440,7 +442,7 @@ sock_connect(VALUE sock, VALUE addr) /* * call-seq: - * socket.connect_nonblock(remote_sockaddr) => 0 + * socket.connect_nonblock(remote_sockaddr, [options]) => 0 * * Requests a connection to be made on the given +remote_sockaddr+ after * O_NONBLOCK is set for the underlying file descriptor. @@ -477,24 +479,36 @@ sock_connect(VALUE sock, VALUE addr) * it is extended by IO::WaitWritable. * So IO::WaitWritable can be used to rescue the exceptions for retrying connect_nonblock. * + * By specifying `exception: false`, the options hash allows you to indicate + * that connect_nonblock should not raise an IO::WaitWritable exception, but + * return the symbol :wait_writable instead. + * * === See * * Socket#connect */ static VALUE -sock_connect_nonblock(VALUE sock, VALUE addr) +sock_connect_nonblock(int argc, VALUE *argv, VALUE sock) { + VALUE addr; + VALUE opts = Qnil; VALUE rai; rb_io_t *fptr; int n; + rb_scan_args(argc, argv, "1:", &addr, &opts); SockAddrStringValueWithAddrinfo(addr, rai); addr = rb_str_new4(addr); GetOpenFile(sock, fptr); rb_io_set_nonblock(fptr); n = connect(fptr->fd, (struct sockaddr*)RSTRING_PTR(addr), RSTRING_SOCKLEN(addr)); if (n < 0) { - if (errno == EINPROGRESS) + if (errno == EINPROGRESS) { + if (!NIL_P(opts) && + Qfalse == rb_hash_lookup2(opts, sym_exception, Qundef)) { + return sym_wait_writable; + } rb_readwrite_sys_fail(RB_IO_WAIT_WRITABLE, "connect(2) would block"); + } rsock_sys_fail_raddrinfo_or_sockaddr("connect(2)", addr, rai); } @@ -2158,7 +2172,7 @@ Init_socket(void) rb_define_method(rb_cSocket, "initialize", sock_initialize, -1); rb_define_method(rb_cSocket, "connect", sock_connect, 1); - rb_define_method(rb_cSocket, "connect_nonblock", sock_connect_nonblock, 1); + rb_define_method(rb_cSocket, "connect_nonblock", sock_connect_nonblock, -1); rb_define_method(rb_cSocket, "bind", sock_bind, 1); rb_define_method(rb_cSocket, "listen", rsock_sock_listen, 1); rb_define_method(rb_cSocket, "accept", sock_accept, 0); @@ -2187,4 +2201,8 @@ Init_socket(void) #endif rb_define_singleton_method(rb_cSocket, "ip_address_list", socket_s_ip_address_list, 0); + +#undef rb_intern + sym_exception = ID2SYM(rb_intern("exception")); + sym_wait_writable = ID2SYM(rb_intern("wait_writable")); } diff --git a/test/openssl/test_pair.rb b/test/openssl/test_pair.rb index 2c2776b644..cfcfa7a74d 100644 --- a/test/openssl/test_pair.rb +++ b/test/openssl/test_pair.rb @@ -283,7 +283,7 @@ module OpenSSL::TestPairM serv.close if serv && !serv.closed? end - def test_accept_nonblock_no_exception + def test_connect_accept_nonblock_no_exception ctx2 = OpenSSL::SSL::SSLContext.new ctx2.ciphers = "ADH" ctx2.tmp_dh_callback = proc { OpenSSL::TestUtils::TEST_KEY_DH1024 } @@ -297,11 +297,31 @@ module OpenSSL::TestPairM ctx1 = OpenSSL::SSL::SSLContext.new ctx1.ciphers = "ADH" s1 = OpenSSL::SSL::SSLSocket.new(sock1, ctx1) - th = Thread.new { s1.connect } + th = Thread.new do + rets = [] + begin + rv = s1.connect_nonblock(exception: false) + rets << rv + case rv + when :wait_writable + IO.select(nil, [s1], nil, 5) + when :wait_readable + IO.select([s1], nil, nil, 5) + end + end until rv == s1 + rets + end + until th.join(0.01) accepted = s2.accept_nonblock(exception: false) assert_includes([s2, :wait_readable, :wait_writable ], accepted) end + + rets = th.value + assert_instance_of Array, rets + rets.each do |rv| + assert_includes([s1, :wait_readable, :wait_writable ], rv) + end ensure s1.close if s1 s2.close if s2 diff --git a/test/socket/test_nonblock.rb b/test/socket/test_nonblock.rb index 94ed198616..6912046879 100644 --- a/test/socket/test_nonblock.rb +++ b/test/socket/test_nonblock.rb @@ -55,6 +55,29 @@ class TestSocketNonblock < Test::Unit::TestCase s.close if s end + def test_connect_nonblock_no_exception + serv = Socket.new(:INET, :STREAM) + serv.bind(Socket.sockaddr_in(0, "127.0.0.1")) + serv.listen(5) + c = Socket.new(:INET, :STREAM) + servaddr = serv.getsockname + rv = c.connect_nonblock(servaddr, exception: false) + case rv + when 0 + # some OSes return immediately on non-blocking local connect() + else + assert_equal :wait_writable, rv + end + assert_equal([ [], [c], [] ], IO.select(nil, [c], nil, 60)) + s, sockaddr = serv.accept + assert_equal(Socket.unpack_sockaddr_in(c.getsockname), + Socket.unpack_sockaddr_in(sockaddr)) + ensure + serv.close if serv + c.close if c + s.close if s + end + def test_udp_recvfrom_nonblock u1 = UDPSocket.new u2 = UDPSocket.new -- cgit v1.2.3