[ruby-core:22807] Re: [Bug #814] NoMethodError: undefined method `read_nonblock' for #<OpenSSL::SSL::SSLSocket:0x1a64f9a0>

In article <[e-mail]>,
  Tony Arcieri <[e-mail]> writes:

>> Does it care the I/O buffer implemented in
>> ext/openssl/lib/openssl/buffering.rb ?
>>
>
> I'm not certain.  What exactly does this do?

Assume gets and read_nonblock is used as follows.

  ssl.gets
  ssl.read_nonblock(10)

Does read_nonblock returns the data just after the line
returned by gets?

gets may read more than a line internally.  The buffer holds
the data after the first line.

If read_nonblock ignores the buffer, the data is not
returned.

write_nonblock has similar issue.  So IO#write_nonblock
flushes the buffer.  It may block, though.

> What is the present behavior if IO#read_nonblock is called when the socket
> needs writable to continue?  How are cases like SSL renegotiation presently
> handled?

IO#read_nonblock doesn't related to SSL.

I assumes you mean OpenSSL::Buffering#read_nonblock.

OpenSSL::Buffering#read_nonblock raises Errno::EWOULDBLOCK
both on SSL_ERROR_WANT_WRITE and SSL_ERROR_WANT_READ.

So it needs to examine the exception message to distinguish
them.  I don't like it.  This is a reason for IO::WantRead
and IO::WantWrite.

> Yes, unfortunately the concerns of nonblocking SSL are different than they
> are from a standard socket.  Standard nonblocking sockets never expect the
> socket need be ready for an I/O operation which wasn't requested.
> Unfortunately the requirements of SSL necessitate waiting on readability or
> writability regardless of which operation was requested.  The socket may
> need to be writable in a case where read_nonblock was requested, and vice
> versa.

I know it.

I read http://www.openssl.org/support/faq.html#PROG10 .

>> I think Errno::EWOULDBLOCK object extended by some module,
>> IO::WandRead or IO::WantWrite, is better.
>>
>
> I can certainly use those exceptions instead.

I implemented IO::WandRead and IO::WantWrite last week.

IO::WandRead and IO::WantWrite is not a class but a module.

Errno::EWOULDBLOCK exception extended by IO::WandRead (or
IO::WantWrite) is compatible with current ruby and
distinguishable with rescue clause.

  begin
    io_or_ssl.read_nonblock(10)
  rescue IO::WantRead
    IO.select([io_or_ssl])
    retry
  rescue IO::WantWrite
    IO.select(nil, [io_or_ssl])
    retry
  end

IO::WantRead and IO::WantWrite also helps non-SSL
nonblocking IO because we don't need to list several
exceptions as Errno::EAGAIN, Errno::EWOULDBLOCK.

It is not committed because Martin said the names are not
good.

% svn diff --diff-cmd diff -x '-u -p'
Index: include/ruby/ruby.h
===================================================================
--- include/ruby/ruby.h	(revision 22872)
+++ include/ruby/ruby.h	(working copy)
@@ -979,6 +979,7 @@ PRINTF_ARGS(NORETURN(void rb_raise(VALUE
 PRINTF_ARGS(NORETURN(void rb_fatal(const char*, ...)), 1, 2);
 PRINTF_ARGS(NORETURN(void rb_bug(const char*, ...)), 1, 2);
 NORETURN(void rb_sys_fail(const char*));
+NORETURN(void rb_mod_sys_fail(VALUE, const char*));
 NORETURN(void rb_iter_break(void));
 NORETURN(void rb_exit(int));
 NORETURN(void rb_notimplement(void));
@@ -1033,6 +1034,8 @@ RUBY_EXTERN VALUE rb_mFileTest;
 RUBY_EXTERN VALUE rb_mGC;
 RUBY_EXTERN VALUE rb_mMath;
 RUBY_EXTERN VALUE rb_mProcess;
+RUBY_EXTERN VALUE rb_mWantRead;
+RUBY_EXTERN VALUE rb_mWantWrite;
 
 RUBY_EXTERN VALUE rb_cBasicObject;
 RUBY_EXTERN VALUE rb_cObject;
Index: io.c
===================================================================
--- io.c	(revision 22872)
+++ io.c	(working copy)
@@ -110,6 +110,8 @@ extern void Init_File(void);
 VALUE rb_cIO;
 VALUE rb_eEOFError;
 VALUE rb_eIOError;
+VALUE rb_mWantRead;
+VALUE rb_mWantWrite;
 
 VALUE rb_stdin, rb_stdout, rb_stderr;
 VALUE rb_deferr;		/* rescue VIM plugin */
@@ -1755,7 +1757,7 @@ io_getpartial(int argc, VALUE *argv, VAL
             if (!nonblock && rb_io_wait_readable(fptr->fd))
                 goto again;
             if (nonblock && errno == EWOULDBLOCK)
-                rb_sys_fail("WANT_READ");
+                rb_mod_sys_fail(rb_mWantRead, "WANT_READ");
             rb_sys_fail_path(fptr->pathv);
         }
 	else if (n == 0) {
@@ -1956,7 +1958,7 @@ rb_io_write_nonblock(VALUE io, VALUE str
 
     if (n == -1) {
         if (errno == EWOULDBLOCK)
-            rb_sys_fail("WANT_WRITE");
+            rb_mod_sys_fail(rb_mWantWrite, "WANT_WRITE");
         rb_sys_fail_path(fptr->pathv);
     }
 
@@ -8635,6 +8637,9 @@ Init_IO(void)
     rb_cIO = rb_define_class("IO", rb_cObject);
     rb_include_module(rb_cIO, rb_mEnumerable);
 
+    rb_mWantRead = rb_define_module_under(rb_cIO, "WantRead");
+    rb_mWantWrite = rb_define_module_under(rb_cIO, "WantWrite");
+
 #if 0
     /* This is necessary only for forcing rdoc handle File::open */
     rb_define_singleton_method(rb_cFile, "open",  rb_io_s_open, -1);
Index: ext/openssl/ossl_ssl.c
===================================================================
--- ext/openssl/ossl_ssl.c	(revision 22872)
+++ ext/openssl/ossl_ssl.c	(working copy)
@@ -1115,14 +1115,14 @@ ossl_ssl_read_internal(int argc, VALUE *
 	    case SSL_ERROR_WANT_WRITE:
                 if (nonblock) {
                     errno = EWOULDBLOCK;
-                    rb_sys_fail("SSL_ERROR_WANT_WRITE");
+                    rb_mod_sys_fail(rb_mWantWrite, "SSL_ERROR_WANT_WRITE");
                 }
                 rb_io_wait_writable(FPTR_TO_FD(fptr));
                 continue;
 	    case SSL_ERROR_WANT_READ:
                 if (nonblock) {
                     errno = EWOULDBLOCK;
-                    rb_sys_fail("SSL_ERROR_WANT_READ");
+                    rb_mod_sys_fail(rb_mWantRead, "SSL_ERROR_WANT_READ");
                 }
                 rb_io_wait_readable(FPTR_TO_FD(fptr));
 		continue;
Index: ext/socket/init.c
===================================================================
--- ext/socket/init.c	(revision 22872)
+++ ext/socket/init.c	(working copy)
@@ -200,7 +200,7 @@ rsock_s_recvfrom_nonblock(VALUE sock, in
 #if defined(EWOULDBLOCK) && EWOULDBLOCK != EAGAIN
 	  case EWOULDBLOCK:
 #endif
-            rb_sys_fail("recvfrom(2) WANT_READ");
+            rb_mod_sys_fail(rb_mWantRead, "recvfrom(2) WANT_READ");
 	}
 	rb_sys_fail("recvfrom(2)");
     }
@@ -472,7 +472,7 @@ rsock_s_accept_nonblock(VALUE klass, rb_
 #if defined EPROTO
 	  case EPROTO:
 #endif
-            rb_sys_fail("accept(2) WANT_READ");
+            rb_mod_sys_fail(rb_mWantRead, "accept(2) WANT_READ");
 	}
         rb_sys_fail("accept(2)");
     }
Index: ext/socket/socket.c
===================================================================
--- ext/socket/socket.c	(revision 22872)
+++ ext/socket/socket.c	(working copy)
@@ -312,7 +312,7 @@ sock_connect_nonblock(VALUE sock, VALUE 
     n = connect(fptr->fd, (struct sockaddr*)RSTRING_PTR(addr), RSTRING_LEN(addr));
     if (n < 0) {
         if (errno == EINPROGRESS)
-            rb_sys_fail("connect(2) WANT_WRITE");
+            rb_mod_sys_fail(rb_mWantWrite, "connect(2) WANT_WRITE");
 	rb_sys_fail("connect(2)");
     }
 
Index: ext/socket/ancdata.c
===================================================================
--- ext/socket/ancdata.c	(revision 22872)
+++ ext/socket/ancdata.c	(working copy)
@@ -1280,7 +1280,7 @@ bsock_sendmsg_internal(int argc, VALUE *
 
     if (ss == -1) {
         if (nonblock && errno == EWOULDBLOCK)
-            rb_sys_fail("sendmsg(2) WANT_WRITE");
+            rb_mod_sys_fail(rb_mWantWrite, "sendmsg(2) WANT_WRITE");
 	rb_sys_fail("sendmsg(2)");
     }
 
@@ -1565,7 +1565,7 @@ bsock_recvmsg_internal(int argc, VALUE *
 
     if (ss == -1) {
         if (nonblock && errno == EWOULDBLOCK)
-            rb_sys_fail("recvmsg(2) WANT_READ");
+            rb_mod_sys_fail(rb_mWantRead, "recvmsg(2) WANT_READ");
 #if defined(HAVE_ST_MSG_CONTROL)
         if (!gc_done && (errno == EMFILE || errno == EMSGSIZE)) {
           /*
Index: error.c
===================================================================
--- error.c	(revision 22872)
+++ error.c	(working copy)
@@ -1129,8 +1129,8 @@ rb_fatal(const char *fmt, ...)
     rb_exc_fatal(rb_exc_new3(rb_eFatal, mesg));
 }
 
-void
-rb_sys_fail(const char *mesg)
+static VALUE
+make_errno_exc(const char *mesg)
 {
     int n = errno;
     VALUE arg;
@@ -1141,7 +1141,21 @@ rb_sys_fail(const char *mesg)
     }
 
     arg = mesg ? rb_str_new2(mesg) : Qnil;
-    rb_exc_raise(rb_class_new_instance(1, &arg, get_syserr(n)));
+    return rb_class_new_instance(1, &arg, get_syserr(n));
+}
+
+void
+rb_sys_fail(const char *mesg)
+{
+    rb_exc_raise(make_errno_exc(mesg));
+}
+
+void
+rb_mod_sys_fail(VALUE mod, const char *mesg)
+{
+    VALUE exc = make_errno_exc(mesg);
+    rb_extend_object(exc, mod);
+    rb_exc_raise(exc);
 }
 
 void
Index: test/openssl/test_pair.rb
===================================================================
--- test/openssl/test_pair.rb	(revision 22872)
+++ test/openssl/test_pair.rb	(working copy)
@@ -147,7 +147,7 @@ class OpenSSL::TestPair < Test::Unit::Te
   def test_read_nonblock
     ssl_pair {|s1, s2|
       err = nil
-      assert_raise(Errno::EWOULDBLOCK) {
+      assert_raise(IO::WantRead) {
         begin
           s2.read_nonblock(10)
         ensure
Index: test/openssl/test_ssl.rb
===================================================================
--- test/openssl/test_ssl.rb	(revision 22872)
+++ test/openssl/test_ssl.rb	(working copy)
@@ -172,12 +172,12 @@ class OpenSSL::TestSSL < Test::Unit::Tes
       ssl = OpenSSL::SSL::SSLSocket.new(sock)
       ssl.sync_close = true
       ssl.connect
-      assert_raise(Errno::EAGAIN, Errno::EWOULDBLOCK) { ssl.read_nonblock(100) }
+      assert_raise(IO::WantRead) { ssl.read_nonblock(100) }
       ssl.write("abc\n")
       IO.select [ssl]
       assert_equal('a', ssl.read_nonblock(1))
       assert_equal("bc\n", ssl.read_nonblock(100))
-      assert_raise(Errno::EAGAIN, Errno::EWOULDBLOCK) { ssl.read_nonblock(100) }
+      assert_raise(IO::WantRead) { ssl.read_nonblock(100) }
     }
   end
 
Index: test/socket/test_nonblock.rb
===================================================================
--- test/socket/test_nonblock.rb	(revision 22872)
+++ test/socket/test_nonblock.rb	(working copy)
@@ -12,13 +12,13 @@ class TestSocketNonblock < Test::Unit::T
     serv = Socket.new(Socket::AF_INET, Socket::SOCK_STREAM, 0)
     serv.bind(Socket.sockaddr_in(0, "127.0.0.1"))
     serv.listen(5)
-    assert_raise(Errno::EAGAIN, Errno::EWOULDBLOCK) { serv.accept_nonblock }
+    assert_raise(IO::WantRead) { serv.accept_nonblock }
     c = Socket.new(Socket::AF_INET, Socket::SOCK_STREAM, 0)
     c.connect(serv.getsockname)
     begin
       s, sockaddr = serv.accept_nonblock
-    rescue Errno::EWOULDBLOCK
-      IO.select nil, [serv]
+    rescue IO::WantRead
+      IO.select [serv]
       s, sockaddr = serv.accept_nonblock
     end
     assert_equal(Socket.unpack_sockaddr_in(c.getsockname), Socket.unpack_sockaddr_in(sockaddr))
@@ -57,8 +57,8 @@ class TestSocketNonblock < Test::Unit::T
     u1 = UDPSocket.new
     u2 = UDPSocket.new
     u1.bind("127.0.0.1", 0)
-    assert_raise(Errno::EAGAIN, Errno::EWOULDBLOCK) { u1.recvfrom_nonblock(100) }
-    assert_raise(Errno::EAGAIN, Errno::EWOULDBLOCK, Errno::EINVAL) { u2.recvfrom_nonblock(100) }
+    assert_raise(IO::WantRead) { u1.recvfrom_nonblock(100) }
+    assert_raise(IO::WantRead, Errno::EINVAL) { u2.recvfrom_nonblock(100) }
     u2.send("aaa", 0, u1.getsockname)
     IO.select [u1]
     mesg, inet_addr = u1.recvfrom_nonblock(100)
@@ -67,7 +67,7 @@ class TestSocketNonblock < Test::Unit::T
     af, port, host, addr = inet_addr
     u2_port, u2_addr = Socket.unpack_sockaddr_in(u2.getsockname)
     assert_equal(u2_port, port)
-    assert_raise(Errno::EAGAIN, Errno::EWOULDBLOCK) { u1.recvfrom_nonblock(100) }
+    assert_raise(IO::WantRead) { u1.recvfrom_nonblock(100) }
     u2.send("", 0, u1.getsockname)
     assert_nothing_raised("cygwin 1.5.19 has a problem to send an empty UDP packet. [ruby-dev:28915]") {
       timeout(1) { IO.select [u1] }
@@ -83,13 +83,13 @@ class TestSocketNonblock < Test::Unit::T
     u1 = UDPSocket.new
     u2 = UDPSocket.new
     u1.bind("127.0.0.1", 0)
-    assert_raise(Errno::EAGAIN, Errno::EWOULDBLOCK) { u1.recv_nonblock(100) }
-    assert_raise(Errno::EAGAIN, Errno::EWOULDBLOCK, Errno::EINVAL) { u2.recv_nonblock(100) }
+    assert_raise(IO::WantRead) { u1.recv_nonblock(100) }
+    assert_raise(IO::WantRead, Errno::EINVAL) { u2.recv_nonblock(100) }
     u2.send("aaa", 0, u1.getsockname)
     IO.select [u1]
     mesg = u1.recv_nonblock(100)
     assert_equal("aaa", mesg)
-    assert_raise(Errno::EAGAIN, Errno::EWOULDBLOCK) { u1.recv_nonblock(100) }
+    assert_raise(IO::WantRead) { u1.recv_nonblock(100) }
     u2.send("", 0, u1.getsockname)
     assert_nothing_raised("cygwin 1.5.19 has a problem to send an empty UDP packet. [ruby-dev:28915]") {
       timeout(1) { IO.select [u1] }
@@ -105,8 +105,8 @@ class TestSocketNonblock < Test::Unit::T
     s1 = Socket.new(Socket::AF_INET, Socket::SOCK_DGRAM, 0)
     s1.bind(Socket.sockaddr_in(0, "127.0.0.1"))
     s2 = Socket.new(Socket::AF_INET, Socket::SOCK_DGRAM, 0)
-    assert_raise(Errno::EAGAIN, Errno::EWOULDBLOCK) { s1.recvfrom_nonblock(100) }
-    assert_raise(Errno::EAGAIN, Errno::EWOULDBLOCK, Errno::EINVAL) { s2.recvfrom_nonblock(100) }
+    assert_raise(IO::WantRead) { s1.recvfrom_nonblock(100) }
+    assert_raise(IO::WantRead, Errno::EINVAL) { s2.recvfrom_nonblock(100) }
     s2.send("aaa", 0, s1.getsockname)
     IO.select [s1]
     mesg, sockaddr = s1.recvfrom_nonblock(100)
@@ -140,13 +140,13 @@ class TestSocketNonblock < Test::Unit::T
 
   def test_tcp_recv_nonblock
     c, s = tcp_pair
-    assert_raise(Errno::EAGAIN, Errno::EWOULDBLOCK) { c.recv_nonblock(100) }
-    assert_raise(Errno::EAGAIN, Errno::EWOULDBLOCK) { s.recv_nonblock(100) }
+    assert_raise(IO::WantRead) { c.recv_nonblock(100) }
+    assert_raise(IO::WantRead) { s.recv_nonblock(100) }
     c.write("abc")
     IO.select [s]
     assert_equal("a", s.recv_nonblock(1))
     assert_equal("bc", s.recv_nonblock(100))
-    assert_raise(Errno::EAGAIN, Errno::EWOULDBLOCK) { s.recv_nonblock(100) }
+    assert_raise(IO::WantRead) { s.recv_nonblock(100) }
   ensure
     c.close if c
     s.close if s
@@ -154,13 +154,13 @@ class TestSocketNonblock < Test::Unit::T
 
   def test_read_nonblock
     c, s = tcp_pair
-    assert_raise(Errno::EAGAIN, Errno::EWOULDBLOCK) { c.read_nonblock(100) }
-    assert_raise(Errno::EAGAIN, Errno::EWOULDBLOCK) { s.read_nonblock(100) }
+    assert_raise(IO::WantRead) { c.read_nonblock(100) }
+    assert_raise(IO::WantRead) { s.read_nonblock(100) }
     c.write("abc")
     IO.select [s]
     assert_equal("a", s.read_nonblock(1))
     assert_equal("bc", s.read_nonblock(100))
-    assert_raise(Errno::EAGAIN, Errno::EWOULDBLOCK) { s.read_nonblock(100) }
+    assert_raise(IO::WantRead) { s.read_nonblock(100) }
   ensure
     c.close if c
     s.close if s
@@ -175,7 +175,7 @@ class TestSocketNonblock < Test::Unit::T
     ret = c.write_nonblock(str)
     assert_operator(ret, :>, 0)
     loop {
-      assert_raise(Errno::EAGAIN, Errno::EWOULDBLOCK) {
+      assert_raise(IO::WantWrite) {
         loop {
           ret = c.write_nonblock(str)
           assert_operator(ret, :>, 0)
@@ -196,7 +196,7 @@ class TestSocketNonblock < Test::Unit::T
         loop {
           c.sendmsg_nonblock("a" * 100000)
         }
-      rescue Errno::EWOULDBLOCK
+      rescue IO::WantWrite
         assert_match(/WANT_WRITE/, $!.message)
       end
     }
@@ -206,7 +206,7 @@ class TestSocketNonblock < Test::Unit::T
     tcp_pair {|c, s|
       begin
         c.recvmsg_nonblock(4096)
-      rescue Errno::EWOULDBLOCK
+      rescue IO::WantRead
         assert_match(/WANT_READ/, $!.message)
       end
     }
@@ -216,7 +216,7 @@ class TestSocketNonblock < Test::Unit::T
     tcp_pair {|c, s|
       begin
         c.recv_nonblock(4096)
-      rescue Errno::EWOULDBLOCK
+      rescue IO::WantRead
         assert_match(/WANT_READ/, $!.message)
       end
     }
@@ -243,7 +243,7 @@ class TestSocketNonblock < Test::Unit::T
     port = serv.local_address.ip_port
     begin
       s, _ = serv.accept_nonblock
-    rescue Errno::EWOULDBLOCK, Errno::ECONNABORTED, Errno::EPROTO
+    rescue IO::WantRead
       assert_match(/WANT_READ/, $!.message)
     end
   ensure
Index: test/socket/test_addrinfo.rb
===================================================================
--- test/socket/test_addrinfo.rb	(revision 22872)
+++ test/socket/test_addrinfo.rb	(working copy)
@@ -236,7 +236,7 @@ class TestSocketAddrinfo < Test::Unit::T
     c.connect(serv.local_address)
     begin
       ret = serv.accept_nonblock
-    rescue Errno::EAGAIN, Errno::EWOULDBLOCK, Errno::ECONNABORTED, Errno::EPROTO, Errno::EINTR
+    rescue IO::WantRead, Errno::EINTR
       IO.select([serv])
       retry
     end
@@ -299,7 +299,7 @@ class TestSocketAddrinfo < Test::Unit::T
     s2.send("test-socket-recvfrom", 0, s1.getsockname)
     begin
       data, ai = s1.recvfrom_nonblock(100)
-    rescue Errno::EWOULDBLOCK
+    rescue IO::WantRead
       IO.select([s1])
       retry
     end
-- 
Tanaka Akira