dumping ground for random patches and texts
 help / color / mirror / Atom feed
* [PATCH 1/2] accept_nonblock supports "exception: false"
@ 2014-11-20 20:17 Eric Wong
  2014-11-20 20:17 ` [PATCH 2/2] openssl: support accept_nonblock(exception: false) Eric Wong
  0 siblings, 1 reply; 2+ messages in thread
From: Eric Wong @ 2014-11-20 20:17 UTC (permalink / raw)
  To: spew

This is analogous to functionality found in IO#read_nonblock and
IO#wait_nonblock.  Raising exceptions for common failures on
non-blocking servers is expensive and makes $DEBUG too noisy.

Benchmark results:
                                    user     system      total        real
default                         2.790000   0.870000   3.660000 (  3.671597)
exception: false                1.120000   0.800000   1.920000 (  1.922032)
exception: false (cached arg)   0.820000   0.770000   1.590000 (  1.589267)
--------------------- benchmark script ------------------------
require 'socket'
require 'benchmark'
require 'tmpdir'
nr = 1000000
Dir.mktmpdir('nb_bench') do |path|
  sock_path = "#{path}/test.sock"
  s = UNIXServer.new(sock_path)
  Benchmark.bmbm do |x|
    x.report("default") do
      nr.times do
        begin
          s.accept_nonblock
        rescue IO::WaitReadable
        end
      end
    end
    x.report("exception: false") do
      nr.times do
        begin
          s.accept_nonblock(exception: false)
        rescue IO::WaitReadable
          abort "should not raise"
        end
      end
    end
    x.report("exception: false (cached arg)") do
      arg = { exception: false }
      nr.times do
        begin
          s.accept_nonblock(arg)
        rescue IO::WaitReadable
          abort "should not raise"
        end
      end
    end
  end
end
---
 ext/socket/init.c            | 18 ++++++++++++++++--
 ext/socket/rubysocket.h      |  2 +-
 ext/socket/socket.c          | 10 +++++++---
 ext/socket/tcpserver.c       |  6 +++---
 ext/socket/unixserver.c      |  6 +++---
 test/socket/test_nonblock.rb |  2 ++
 test/socket/test_tcp.rb      |  7 +++++++
 test/socket/test_unix.rb     |  7 +++++++
 8 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/ext/socket/init.c b/ext/socket/init.c
index c3f3396..b94b250 100644
--- a/ext/socket/init.c
+++ b/ext/socket/init.c
@@ -29,6 +29,7 @@ VALUE rb_cSOCKSSocket;
 #endif
 
 int rsock_do_not_reverse_lookup = 1;
+static VALUE sym_exception, sym_wait_readable;
 
 void
 rsock_raise_socket_error(const char *reason, int error)
@@ -505,11 +506,18 @@ cloexec_accept(int socket, struct sockaddr *address, socklen_t *address_len)
     return ret;
 }
 
-
 VALUE
-rsock_s_accept_nonblock(VALUE klass, rb_io_t *fptr, struct sockaddr *sockaddr, socklen_t *len)
+rsock_s_accept_nonblock(int argc, VALUE *argv, VALUE klass, rb_io_t *fptr,
+			struct sockaddr *sockaddr, socklen_t *len)
 {
     int fd2;
+    int ex = 1;
+    VALUE opts = Qnil;
+
+    rb_scan_args(argc, argv, "0:", &opts);
+
+    if (!NIL_P(opts) && Qfalse == rb_hash_aref(opts, sym_exception))
+	ex = 0;
 
     rb_secure(3);
     rb_io_set_nonblock(fptr);
@@ -524,6 +532,8 @@ rsock_s_accept_nonblock(VALUE klass, rb_io_t *fptr, struct sockaddr *sockaddr, s
 #if defined EPROTO
 	  case EPROTO:
 #endif
+            if (!ex)
+		return sym_wait_readable;
             rb_readwrite_sys_fail(RB_IO_WAIT_READABLE, "accept(2) would block");
 	}
         rb_sys_fail("accept(2)");
@@ -612,4 +622,8 @@ rsock_init_socket_init(void)
     rsock_init_addrinfo();
     rsock_init_sockifaddr();
     rsock_init_socket_constants();
+
+#undef rb_intern
+    sym_exception = ID2SYM(rb_intern("exception"));
+    sym_wait_readable = ID2SYM(rb_intern("wait_readable"));
 }
diff --git a/ext/socket/rubysocket.h b/ext/socket/rubysocket.h
index 8beffd2..689a7a8 100644
--- a/ext/socket/rubysocket.h
+++ b/ext/socket/rubysocket.h
@@ -343,7 +343,7 @@ VALUE rsock_s_recvfrom(VALUE sock, int argc, VALUE *argv, enum sock_recv_type fr
 int rsock_connect(int fd, const struct sockaddr *sockaddr, int len, int socks);
 
 VALUE rsock_s_accept(VALUE klass, int fd, struct sockaddr *sockaddr, socklen_t *len);
-VALUE rsock_s_accept_nonblock(VALUE klass, rb_io_t *fptr, struct sockaddr *sockaddr, socklen_t *len);
+VALUE rsock_s_accept_nonblock(int argc, VALUE *argv, VALUE klass, rb_io_t *fptr, struct sockaddr *sockaddr, socklen_t *len);
 VALUE rsock_sock_listen(VALUE sock, VALUE log);
 
 VALUE rsock_sockopt_new(int family, int level, int optname, VALUE data);
diff --git a/ext/socket/socket.c b/ext/socket/socket.c
index 81028c5..587e9f8 100644
--- a/ext/socket/socket.c
+++ b/ext/socket/socket.c
@@ -944,15 +944,19 @@ sock_accept(VALUE sock)
  * * Socket#accept
  */
 static VALUE
-sock_accept_nonblock(VALUE sock)
+sock_accept_nonblock(int argc, VALUE *argv, VALUE sock)
 {
     rb_io_t *fptr;
     VALUE sock2;
     union_sockaddr buf;
+    struct sockaddr *addr = &buf.addr;
     socklen_t len = (socklen_t)sizeof buf;
 
     GetOpenFile(sock, fptr);
-    sock2 = rsock_s_accept_nonblock(rb_cSocket, fptr, &buf.addr, &len);
+    sock2 = rsock_s_accept_nonblock(argc, argv, rb_cSocket, fptr, addr, &len);
+
+    if (SYMBOL_P(sock2)) /* :wait_readable */
+	return sock2;
     return rb_assoc_new(sock2, rsock_io_socket_addrinfo(sock2, &buf.addr, len));
 }
 
@@ -2154,7 +2158,7 @@ Init_socket(void)
     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);
-    rb_define_method(rb_cSocket, "accept_nonblock", sock_accept_nonblock, 0);
+    rb_define_method(rb_cSocket, "accept_nonblock", sock_accept_nonblock, -1);
     rb_define_method(rb_cSocket, "sysaccept", sock_sysaccept, 0);
 
     rb_define_method(rb_cSocket, "recvfrom", sock_recvfrom, -1);
diff --git a/ext/socket/tcpserver.c b/ext/socket/tcpserver.c
index 2245a06..9515c13 100644
--- a/ext/socket/tcpserver.c
+++ b/ext/socket/tcpserver.c
@@ -98,7 +98,7 @@ tcp_accept(VALUE sock)
  * * Socket#accept
  */
 static VALUE
-tcp_accept_nonblock(VALUE sock)
+tcp_accept_nonblock(int argc, VALUE *argv, VALUE sock)
 {
     rb_io_t *fptr;
     union_sockaddr from;
@@ -106,7 +106,7 @@ tcp_accept_nonblock(VALUE sock)
 
     GetOpenFile(sock, fptr);
     fromlen = (socklen_t)sizeof(from);
-    return rsock_s_accept_nonblock(rb_cTCPSocket, fptr, &from.addr, &fromlen);
+    return rsock_s_accept_nonblock(argc, argv, rb_cTCPSocket, fptr, &from.addr, &fromlen);
 }
 
 /*
@@ -171,7 +171,7 @@ rsock_init_tcpserver(void)
      */
     rb_cTCPServer = rb_define_class("TCPServer", rb_cTCPSocket);
     rb_define_method(rb_cTCPServer, "accept", tcp_accept, 0);
-    rb_define_method(rb_cTCPServer, "accept_nonblock", tcp_accept_nonblock, 0);
+    rb_define_method(rb_cTCPServer, "accept_nonblock", tcp_accept_nonblock, -1);
     rb_define_method(rb_cTCPServer, "sysaccept", tcp_sysaccept, 0);
     rb_define_method(rb_cTCPServer, "initialize", tcp_svr_init, -1);
     rb_define_method(rb_cTCPServer, "listen", rsock_sock_listen, 1); /* in socket.c */
diff --git a/ext/socket/unixserver.c b/ext/socket/unixserver.c
index df98497..6857688 100644
--- a/ext/socket/unixserver.c
+++ b/ext/socket/unixserver.c
@@ -91,7 +91,7 @@ unix_accept(VALUE sock)
  * * Socket#accept
  */
 static VALUE
-unix_accept_nonblock(VALUE sock)
+unix_accept_nonblock(int argc, VALUE *argv, VALUE sock)
 {
     rb_io_t *fptr;
     struct sockaddr_un from;
@@ -99,7 +99,7 @@ unix_accept_nonblock(VALUE sock)
 
     GetOpenFile(sock, fptr);
     fromlen = (socklen_t)sizeof(from);
-    return rsock_s_accept_nonblock(rb_cUNIXSocket, fptr,
+    return rsock_s_accept_nonblock(argc, argv, rb_cUNIXSocket, fptr,
 			           (struct sockaddr *)&from, &fromlen);
 }
 
@@ -148,7 +148,7 @@ rsock_init_unixserver(void)
     rb_cUNIXServer = rb_define_class("UNIXServer", rb_cUNIXSocket);
     rb_define_method(rb_cUNIXServer, "initialize", unix_svr_init, 1);
     rb_define_method(rb_cUNIXServer, "accept", unix_accept, 0);
-    rb_define_method(rb_cUNIXServer, "accept_nonblock", unix_accept_nonblock, 0);
+    rb_define_method(rb_cUNIXServer, "accept_nonblock", unix_accept_nonblock, -1);
     rb_define_method(rb_cUNIXServer, "sysaccept", unix_sysaccept, 0);
     rb_define_method(rb_cUNIXServer, "listen", rsock_sock_listen, 1); /* in socket.c */
 #endif
diff --git a/test/socket/test_nonblock.rb b/test/socket/test_nonblock.rb
index 882e438..94ed198 100644
--- a/test/socket/test_nonblock.rb
+++ b/test/socket/test_nonblock.rb
@@ -13,6 +13,8 @@ class TestSocketNonblock < Test::Unit::TestCase
     serv.bind(Socket.sockaddr_in(0, "127.0.0.1"))
     serv.listen(5)
     assert_raise(IO::WaitReadable) { serv.accept_nonblock }
+    assert_equal :wait_readable, serv.accept_nonblock(exception: false)
+    assert_raise(IO::WaitReadable) { serv.accept_nonblock(exception: true) }
     c = Socket.new(Socket::AF_INET, Socket::SOCK_STREAM, 0)
     c.connect(serv.getsockname)
     begin
diff --git a/test/socket/test_tcp.rb b/test/socket/test_tcp.rb
index 7328897..086d31b 100644
--- a/test/socket/test_tcp.rb
+++ b/test/socket/test_tcp.rb
@@ -76,4 +76,11 @@ class TestSocket_TCPSocket < Test::Unit::TestCase
       th.join
     }
   end
+
+  def test_accept_nonblock
+    svr = TCPServer.new("localhost", 0)
+    assert_raises(IO::WaitReadable) { svr.accept_nonblock }
+    assert_equal :wait_readable, svr.accept_nonblock(exception: false)
+    assert_raises(IO::WaitReadable) { svr.accept_nonblock(exception: true) }
+  end
 end if defined?(TCPSocket)
diff --git a/test/socket/test_unix.rb b/test/socket/test_unix.rb
index 866c839..829eaa9 100644
--- a/test/socket/test_unix.rb
+++ b/test/socket/test_unix.rb
@@ -663,4 +663,11 @@ class TestSocket_UNIXSocket < Test::Unit::TestCase
     assert(s0.closed?)
   end
 
+  def test_accept_nonblock
+    bound_unix_socket(UNIXServer) {|serv, path|
+      assert_raises(IO::WaitReadable) { serv.accept_nonblock }
+      assert_raises(IO::WaitReadable) { serv.accept_nonblock(exception: true) }
+      assert_equal :wait_readable, serv.accept_nonblock(exception: false)
+    }
+  end
 end if defined?(UNIXSocket) && /cygwin/ !~ RUBY_PLATFORM
-- 
EW


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [PATCH 2/2] openssl: support accept_nonblock(exception: false)
  2014-11-20 20:17 [PATCH 1/2] accept_nonblock supports "exception: false" Eric Wong
@ 2014-11-20 20:17 ` Eric Wong
  0 siblings, 0 replies; 2+ messages in thread
From: Eric Wong @ 2014-11-20 20:17 UTC (permalink / raw)
  To: spew

For consistency with [Feature #10532]
---
 ext/openssl/ossl_ssl.c    | 25 ++++++++++++++++++-------
 test/openssl/test_pair.rb | 26 ++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c
index ccfd72d..6b166de 100644
--- a/ext/openssl/ossl_ssl.c
+++ b/ext/openssl/ossl_ssl.c
@@ -1250,7 +1250,8 @@ read_would_block(int nonblock)
 }
 
 static VALUE
-ossl_start_ssl(VALUE self, int (*func)(), const char *funcname, int nonblock)
+ossl_start_ssl(VALUE self, int (*func)(), const char *funcname,
+		int nonblock, int no_exception)
 {
     SSL *ssl;
     rb_io_t *fptr;
@@ -1274,10 +1275,12 @@ ossl_start_ssl(VALUE self, int (*func)(), const char *funcname, int nonblock)
 
 	switch((ret2 = ssl_get_error(ssl, ret))){
 	case SSL_ERROR_WANT_WRITE:
+            if (no_exception) { return ID2SYM(rb_intern("wait_writable")); }
             write_would_block(nonblock);
             rb_io_wait_writable(FPTR_TO_FD(fptr));
             continue;
 	case SSL_ERROR_WANT_READ:
+            if (no_exception) { return ID2SYM(rb_intern("wait_readable")); }
             read_would_block(nonblock);
             rb_io_wait_readable(FPTR_TO_FD(fptr));
             continue;
@@ -1303,7 +1306,7 @@ static VALUE
 ossl_ssl_connect(VALUE self)
 {
     ossl_ssl_setup(self);
-    return ossl_start_ssl(self, SSL_connect, "SSL_connect", 0);
+    return ossl_start_ssl(self, SSL_connect, "SSL_connect", 0, 0);
 }
 
 /*
@@ -1328,7 +1331,7 @@ static VALUE
 ossl_ssl_connect_nonblock(VALUE self)
 {
     ossl_ssl_setup(self);
-    return ossl_start_ssl(self, SSL_connect, "SSL_connect", 1);
+    return ossl_start_ssl(self, SSL_connect, "SSL_connect", 1, 0);
 }
 
 /*
@@ -1342,7 +1345,7 @@ static VALUE
 ossl_ssl_accept(VALUE self)
 {
     ossl_ssl_setup(self);
-    return ossl_start_ssl(self, SSL_accept, "SSL_accept", 0);
+    return ossl_start_ssl(self, SSL_accept, "SSL_accept", 0, 0);
 }
 
 /*
@@ -1364,10 +1367,18 @@ ossl_ssl_accept(VALUE self)
  *
  */
 static VALUE
-ossl_ssl_accept_nonblock(VALUE self)
+ossl_ssl_accept_nonblock(int argc, VALUE *argv, VALUE self)
 {
+    int no_exception = 0;
+    VALUE opts = Qnil;
+
+    rb_scan_args(argc, argv, "0:", &opts);
+
+    if (!NIL_P(opts) && Qfalse == rb_hash_aref(opts, sym_exception))
+	no_exception = 1;
+
     ossl_ssl_setup(self);
-    return ossl_start_ssl(self, SSL_accept, "SSL_accept", 1);
+    return ossl_start_ssl(self, SSL_accept, "SSL_accept", 1, no_exception);
 }
 
 static VALUE
@@ -2199,7 +2210,7 @@ Init_ossl_ssl(void)
     rb_define_method(cSSLSocket, "connect",    ossl_ssl_connect, 0);
     rb_define_method(cSSLSocket, "connect_nonblock",    ossl_ssl_connect_nonblock, 0);
     rb_define_method(cSSLSocket, "accept",     ossl_ssl_accept, 0);
-    rb_define_method(cSSLSocket, "accept_nonblock",     ossl_ssl_accept_nonblock, 0);
+    rb_define_method(cSSLSocket, "accept_nonblock", ossl_ssl_accept_nonblock, -1);
     rb_define_method(cSSLSocket, "sysread",    ossl_ssl_read, -1);
     rb_define_private_method(cSSLSocket, "sysread_nonblock",    ossl_ssl_read_nonblock, -1);
     rb_define_method(cSSLSocket, "syswrite",   ossl_ssl_write, 1);
diff --git a/test/openssl/test_pair.rb b/test/openssl/test_pair.rb
index b9206da..2ca92fe 100644
--- a/test/openssl/test_pair.rb
+++ b/test/openssl/test_pair.rb
@@ -283,6 +283,32 @@ module OpenSSL::TestPairM
     serv.close if serv && !serv.closed?
   end
 
+  def test_accept_nonblock_no_exception
+    ctx2 = OpenSSL::SSL::SSLContext.new
+    ctx2.ciphers = "ADH"
+    ctx2.tmp_dh_callback = proc { OpenSSL::TestUtils::TEST_KEY_DH1024 }
+
+    sock1, sock2 = tcp_pair
+
+    s2 = OpenSSL::SSL::SSLSocket.new(sock2, ctx2)
+    accepted = s2.accept_nonblock(exception: false)
+    assert_equal :wait_readable, accepted
+
+    ctx1 = OpenSSL::SSL::SSLContext.new
+    ctx1.ciphers = "ADH"
+    s1 = OpenSSL::SSL::SSLSocket.new(sock1, ctx1)
+    th = Thread.new { s1.connect }
+    until th.join(0.01)
+      accepted = s2.accept_nonblock(exception: false)
+      assert_includes([s2, :wait_readable, :wait_writable ], accepted)
+    end
+  ensure
+    s1.close if s1 && !s1.closed?
+    s2.close if s2 && !s2.closed?
+    sock1.close if sock1 && !sock1.closed?
+    sock2.close if sock2 && !sock2.closed?
+  end
+
   def test_connect_accept_nonblock
     ctx = OpenSSL::SSL::SSLContext.new()
     ctx.ciphers = "ADH"
-- 
EW


^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2014-11-20 20:17 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-20 20:17 [PATCH 1/2] accept_nonblock supports "exception: false" Eric Wong
2014-11-20 20:17 ` [PATCH 2/2] openssl: support accept_nonblock(exception: false) Eric Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).