BasicSocket#recv* return `nil` rather than an empty packet

[Bug #19012]

man recvmsg(2) states:

> Return Value
> These calls return the number of bytes received, or -1 if an error occurred.
> The return value will be 0 when the peer has performed an orderly shutdown.

Not too sure how one is supposed to make the difference between a packet of
size 0 and a closed connection.

ext/socket/ancdata.c

@@ -1555,6 +1555,10 @@ bsock_recvmsg_internal(VALUE sock,
 
     ss = rb_recvmsg(fptr->fd, &mh, flags);
 
+    if (ss == 0 && !rsock_is_dgram(fptr)) {
+        return Qnil;
+    }
+
     if (ss == -1) {
         int e;
         if (!nonblock && rb_io_maybe_wait_readable(errno, fptr->self, RUBY_IO_TIMEOUT_DEFAULT)) {

ext/socket/init.c

@@ -116,6 +116,7 @@ recvfrom_blocking(void *data)
     ssize_t ret;
     ret = recvfrom(arg->fd, RSTRING_PTR(arg->str), arg->length,
                    arg->flags, &arg->buf.addr, &arg->alen);
+
     if (ret != -1 && len0 < arg->alen)
         arg->alen = len0;
 
@@ -147,6 +148,18 @@ recvfrom_locktmp(VALUE v)
     return rb_thread_io_blocking_region(recvfrom_blocking, arg, arg->fd);
 }
 
+int
+rsock_is_dgram(rb_io_t *fptr)
+{
+    int socktype;
+    socklen_t optlen = (socklen_t)sizeof(socktype);
+    int ret = getsockopt(fptr->fd, SOL_SOCKET, SO_TYPE, (void*)&socktype, &optlen);
+    if (ret == -1) {
+        rb_sys_fail("getsockopt(SO_TYPE)");
+    }
+    return socktype == SOCK_DGRAM;
+}
+
 VALUE
 rsock_s_recvfrom(VALUE socket, int argc, VALUE *argv, enum sock_recv_type from)
 {
@@ -187,6 +200,9 @@ rsock_s_recvfrom(VALUE socket, int argc, VALUE *argv, enum sock_recv_type from)
 
         slen = (long)rb_str_locktmp_ensure(str, recvfrom_locktmp, (VALUE)&arg);
 
+        if (slen == 0 && !rsock_is_dgram(fptr)) {
+            return Qnil;
+        }
         if (slen >= 0) break;
 
         if (!rb_io_maybe_wait_readable(errno, socket, RUBY_IO_TIMEOUT_DEFAULT))
@@ -259,6 +275,10 @@ rsock_s_recvfrom_nonblock(VALUE sock, VALUE len, VALUE flg, VALUE str,
     if (slen != -1 && len0 < alen)
         alen = len0;
 
+    if (slen == 0 && !rsock_is_dgram(fptr)) {
+        return Qnil;
+    }
+
     if (slen < 0) {
         int e = errno;
         switch (e) {

ext/socket/rubysocket.h

@@ -459,6 +459,8 @@ VALUE rsock_write_nonblock(VALUE sock, VALUE buf, VALUE ex);
 
 void rsock_make_fd_nonblock(int fd);
 
+int rsock_is_dgram(rb_io_t *fptr);
+
 #if !defined HAVE_INET_NTOP && ! defined _WIN32
 const char *inet_ntop(int, const void *, char *, size_t);
 #elif defined __MINGW32__

spec/ruby/library/socket/basicsocket/recv_spec.rb

@@ -32,6 +32,25 @@ describe "BasicSocket#recv" do
     ScratchPad.recorded.should == 'hello'
   end
 
+  ruby_version_is "3.3" do
+    it "returns nil on a closed stream socket" do
+      t = Thread.new do
+        client = @server.accept
+        packet = client.recv(10)
+        client.close
+        packet
+      end
+
+      Thread.pass while t.status and t.status != "sleep"
+      t.status.should_not be_nil
+
+      socket = TCPSocket.new('127.0.0.1', @port)
+      socket.close
+
+      t.value.should be_nil
+    end
+  end
+
   platform_is_not :solaris do
     it "accepts flags to specify unusual receiving behaviour" do
       t = Thread.new do

spec/ruby/library/socket/basicsocket/send_spec.rb

@@ -22,7 +22,7 @@ describe "BasicSocket#send" do
        client = @server.accept
        loop do
          got = client.recv(5)
-         break if got.empty?
+         break if got.nil? || got.empty?
          data << got
        end
        client.close
@@ -67,7 +67,7 @@ describe "BasicSocket#send" do
        client = @server.accept
        loop do
          got = client.recv(5)
-         break if got.empty?
+         break if got.nil? || got.empty?
          data << got
        end
        client.close

spec/ruby/library/socket/basicsocket/shutdown_spec.rb

@@ -23,7 +23,7 @@ platform_is_not :windows do # hangs
         it 'shuts down a socket for reading' do
           @client.shutdown(Socket::SHUT_RD)
 
-          @client.recv(1).should be_empty
+          @client.recv(1).to_s.should be_empty
         end
 
         it 'shuts down a socket for writing' do
@@ -35,7 +35,7 @@ platform_is_not :windows do # hangs
         it 'shuts down a socket for reading and writing' do
           @client.shutdown(Socket::SHUT_RDWR)
 
-          @client.recv(1).should be_empty
+          @client.recv(1).to_s.should be_empty
 
           -> { @client.write('hello') }.should raise_error(Errno::EPIPE)
         end
@@ -49,13 +49,13 @@ platform_is_not :windows do # hangs
         it 'shuts down a socket for reading using :RD' do
           @client.shutdown(:RD)
 
-          @client.recv(1).should be_empty
+          @client.recv(1).to_s.should be_empty
         end
 
         it 'shuts down a socket for reading using :SHUT_RD' do
           @client.shutdown(:SHUT_RD)
 
-          @client.recv(1).should be_empty
+          @client.recv(1).to_s.should be_empty
         end
 
         it 'shuts down a socket for writing using :WR' do
@@ -73,7 +73,7 @@ platform_is_not :windows do # hangs
         it 'shuts down a socket for reading and writing' do
           @client.shutdown(:RDWR)
 
-          @client.recv(1).should be_empty
+          @client.recv(1).to_s.should be_empty
 
           -> { @client.write('hello') }.should raise_error(Errno::EPIPE)
         end
@@ -87,13 +87,13 @@ platform_is_not :windows do # hangs
         it 'shuts down a socket for reading using "RD"' do
           @client.shutdown('RD')
 
-          @client.recv(1).should be_empty
+          @client.recv(1).to_s.should be_empty
         end
 
         it 'shuts down a socket for reading using "SHUT_RD"' do
           @client.shutdown('SHUT_RD')
 
-          @client.recv(1).should be_empty
+          @client.recv(1).to_s.should be_empty
         end
 
         it 'shuts down a socket for writing using "WR"' do
@@ -123,7 +123,7 @@ platform_is_not :windows do # hangs
 
           @client.shutdown(@dummy)
 
-          @client.recv(1).should be_empty
+          @client.recv(1).to_s.should be_empty
         end
 
         it 'shuts down a socket for reading using "SHUT_RD"' do
@@ -131,7 +131,7 @@ platform_is_not :windows do # hangs
 
           @client.shutdown(@dummy)
 
-          @client.recv(1).should be_empty
+          @client.recv(1).to_s.should be_empty
         end
 
         it 'shuts down a socket for reading and writing' do
@@ -139,7 +139,7 @@ platform_is_not :windows do # hangs
 
           @client.shutdown(@dummy)
 
-          @client.recv(1).should be_empty
+          @client.recv(1).to_s.should be_empty
 
           -> { @client.write('hello') }.should raise_error(Errno::EPIPE)
         end

spec/ruby/library/socket/fixtures/classes.rb

@@ -113,7 +113,7 @@ module SocketSpecs
       begin
         data = socket.recv(1024)
 
-        return if data.empty?
+        return if data.nil? || data.empty?
         log "SpecTCPServer received: #{data.inspect}"
 
         return if data == "QUIT"

test/socket/test_unix.rb

@@ -420,9 +420,10 @@ class TestSocket_UNIXSocket < Test::Unit::TestCase
       s1.recv_nonblock(10)
       fail
     rescue => e
-      assert(IO::EAGAINWaitReadable === e)
-      assert(IO::WaitReadable === e)
+      assert_kind_of(IO::EWOULDBLOCKWaitReadable, e)
+      assert_kind_of(IO::WaitReadable, e)
     end
+
     s2.send("", 0)
     s2.send("haha", 0)
     s2.send("", 0)
@@ -446,6 +447,69 @@ class TestSocket_UNIXSocket < Test::Unit::TestCase
     s2.close if s2
   end
 
+  def test_stream_pair
+    s1, s2 = UNIXSocket.pair(Socket::SOCK_STREAM)
+    begin
+      s1.recv_nonblock(10)
+      fail
+    rescue => e
+      assert_kind_of(IO::EWOULDBLOCKWaitReadable, e)
+      assert_kind_of(IO::WaitReadable, e)
+    end
+
+    s2.send("", 0)
+    s2.send("haha", 0)
+    assert_equal("haha", s1.recv(10))
+    assert_raise(IO::EWOULDBLOCKWaitReadable) { s1.recv_nonblock(10) }
+
+    buf = "".dup
+    s2.send("BBBBBB", 0)
+    IO.select([s1])
+    rv = s1.recv(100, 0, buf)
+    assert_equal buf.object_id, rv.object_id
+    assert_equal "BBBBBB", rv
+
+    s2.close
+    assert_nil(s1.recv(10))
+  rescue Errno::EPROTOTYPE => error
+    omit error.message
+  ensure
+    s1.close if s1
+    s2.close if s2
+  end
+
+  def test_seqpacket_pair
+    s1, s2 = UNIXSocket.pair(Socket::SOCK_SEQPACKET)
+    begin
+      s1.recv_nonblock(10)
+      fail
+    rescue => e
+      assert_kind_of(IO::EWOULDBLOCKWaitReadable, e)
+      assert_kind_of(IO::WaitReadable, e)
+    end
+
+    s2.send("", 0)
+    s2.send("haha", 0)
+    assert_equal(nil, s1.recv(10)) # no way to distinguish empty packet from EOF with SOCK_SEQPACKET
+    assert_equal("haha", s1.recv(10))
+    assert_raise(IO::EWOULDBLOCKWaitReadable) { s1.recv_nonblock(10) }
+
+    buf = "".dup
+    s2.send("BBBBBB", 0)
+    IO.select([s1])
+    rv = s1.recv(100, 0, buf)
+    assert_equal buf.object_id, rv.object_id
+    assert_equal "BBBBBB", rv
+
+    s2.close
+    assert_nil(s1.recv(10))
+  rescue Errno::EPROTOTYPE, Errno::EPROTONOSUPPORT => error
+    omit error.message
+  ensure
+    s1.close if s1
+    s2.close if s2
+  end
+
   def test_dgram_pair_sendrecvmsg_errno_set
     if /mswin|mingw/ =~ RUBY_PLATFORM
       omit("AF_UNIX + SOCK_DGRAM is not supported on windows")