* ext/socket/unixsocket.c (rsock_init_unixsock): Open a socket

  after path length check.
  This fixes a fd leak by TestSocket_UNIXSocket#test_too_long_path.



git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@46218 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

5 files changed, 143 insertions, 99 deletions

diff --git a/ChangeLog b/ChangeLog
index af80895d0b..1df5d5b606 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+Thu May 29 00:28:56 2014  Tanaka Akira  <akr@fsij.org>
+
+	* ext/socket/unixsocket.c (rsock_init_unixsock): Open a socket
+	  after path length check.
+	  This fixes a fd leak by TestSocket_UNIXSocket#test_too_long_path.
+
 Wed May 28 23:04:35 2014  Tanaka Akira  <akr@fsij.org>
 
 	* test/ruby/test_io.rb (test_flush_in_finalizer1): Use
diff --git a/ext/socket/unixsocket.c b/ext/socket/unixsocket.c
index 519343b558..0a1b65a5b9 100644
--- a/ext/socket/unixsocket.c
+++ b/ext/socket/unixsocket.c
@@ -34,10 +34,6 @@ rsock_init_unixsock(VALUE sock, VALUE path, int server)
     rb_io_t *fptr;
 
     SafeStringValue(path);
-    fd = rsock_socket(AF_UNIX, SOCK_STREAM, 0);
-    if (fd < 0) {
-	rsock_sys_fail_path("socket(2)", path);
-    }
 
     INIT_SOCKADDR_UN(&sockaddr, sizeof(struct sockaddr_un));
     if (sizeof(sockaddr.sun_path) < (size_t)RSTRING_LEN(path)) {
@@ -47,6 +43,11 @@ rsock_init_unixsock(VALUE sock, VALUE path, int server)
     memcpy(sockaddr.sun_path, RSTRING_PTR(path), RSTRING_LEN(path));
     sockaddrlen = rsock_unix_sockaddr_len(path);
 
+    fd = rsock_socket(AF_UNIX, SOCK_STREAM, 0);
+    if (fd < 0) {
+	rsock_sys_fail_path("socket(2)", path);
+    }
+
     if (server) {
         status = bind(fd, (struct sockaddr*)&sockaddr, sockaddrlen);
     }
diff --git a/test/socket/test_socket.rb b/test/socket/test_socket.rb
index 6e4680cd7f..1db0ea251d 100644
--- a/test/socket/test_socket.rb
+++ b/test/socket/test_socket.rb
@@ -538,6 +538,7 @@ class TestSocket < Test::Unit::TestCase
       assert_raise(IOError, bug4390) {client_thread.join}
     end
   ensure
+    serv_thread.value.close
     server.close
   end
 
diff --git a/test/socket/test_tcp.rb b/test/socket/test_tcp.rb
index 73747929e3..7328897d1e 100644
--- a/test/socket/test_tcp.rb
+++ b/test/socket/test_tcp.rb
@@ -45,36 +45,35 @@ class TestSocket_TCPSocket < Test::Unit::TestCase
   end
 
   def test_recvfrom
-    svr = TCPServer.new("localhost", 0)
-    th = Thread.new {
-      c = svr.accept
-      c.write "foo"
-      c.close
+    TCPServer.open("localhost", 0) {|svr|
+      th = Thread.new {
+        c = svr.accept
+        c.write "foo"
+        c.close
+      }
+      addr = svr.addr
+      TCPSocket.open(addr[3], addr[1]) {|sock|
+        assert_equal(["foo", nil], sock.recvfrom(0x10000))
+      }
+      th.join
     }
-    addr = svr.addr
-    sock = TCPSocket.open(addr[3], addr[1])
-    assert_equal(["foo", nil], sock.recvfrom(0x10000))
-  ensure
-    th.kill if th
-    th.join if th
   end
 
   def test_encoding
-    svr = TCPServer.new("localhost", 0)
-    th = Thread.new {
-      c = svr.accept
-      c.write "foo\r\n"
-      c.close
+    TCPServer.open("localhost", 0) {|svr|
+      th = Thread.new {
+        c = svr.accept
+        c.write "foo\r\n"
+        c.close
+      }
+      addr = svr.addr
+      TCPSocket.open(addr[3], addr[1]) {|sock|
+        assert_equal(true, sock.binmode?)
+        s = sock.gets
+        assert_equal("foo\r\n", s)
+        assert_equal(Encoding.find("ASCII-8BIT"), s.encoding)
+      }
+      th.join
     }
-    addr = svr.addr
-    sock = TCPSocket.open(addr[3], addr[1])
-    assert_equal(true, sock.binmode?)
-    s = sock.gets
-    assert_equal("foo\r\n", s)
-    assert_equal(Encoding.find("ASCII-8BIT"), s.encoding)
-  ensure
-    th.kill if th
-    th.join if th
-    sock.close if sock
   end
 end if defined?(TCPSocket)
diff --git a/test/socket/test_unix.rb b/test/socket/test_unix.rb
index a6879bbfe4..5583cb6d9b 100644
--- a/test/socket/test_unix.rb
+++ b/test/socket/test_unix.rb
@@ -55,16 +55,20 @@ class TestSocket_UNIXSocket < Test::Unit::TestCase
         ret = s2.recvmsg(:scm_rights=>true)
         _, _, _, *ctls = ret
         recv_io_ary = []
-        ctls.each {|ctl|
-          next if ctl.level != Socket::SOL_SOCKET || ctl.type != Socket::SCM_RIGHTS
-          recv_io_ary.concat ctl.unix_rights
-        }
-        assert_equal(send_io_ary.length, recv_io_ary.length)
-        send_io_ary.length.times {|i|
-          assert_not_equal(send_io_ary[i].fileno, recv_io_ary[i].fileno)
-          assert(File.identical?(send_io_ary[i], recv_io_ary[i]))
-          assert(recv_io_ary[i].close_on_exec?)
-        }
+        begin
+          ctls.each {|ctl|
+            next if ctl.level != Socket::SOL_SOCKET || ctl.type != Socket::SCM_RIGHTS
+            recv_io_ary.concat ctl.unix_rights
+          }
+          assert_equal(send_io_ary.length, recv_io_ary.length)
+          send_io_ary.length.times {|i|
+            assert_not_equal(send_io_ary[i].fileno, recv_io_ary[i].fileno)
+            assert(File.identical?(send_io_ary[i], recv_io_ary[i]))
+            assert(recv_io_ary[i].close_on_exec?)
+          }
+        ensure
+          recv_io_ary.each {|io| io.close if !io.closed? }
+        end
       }
     }
   ensure
@@ -92,16 +96,20 @@ class TestSocket_UNIXSocket < Test::Unit::TestCase
         ret = s2.recvmsg(:scm_rights=>true)
         _, _, _, *ctls = ret
         recv_io_ary = []
-        ctls.each {|ctl|
-          next if ctl.level != Socket::SOL_SOCKET || ctl.type != Socket::SCM_RIGHTS
-          recv_io_ary.concat ctl.unix_rights
-        }
-        assert_equal(send_io_ary.length, recv_io_ary.length)
-        send_io_ary.length.times {|i|
-          assert_not_equal(send_io_ary[i].fileno, recv_io_ary[i].fileno)
-          assert(File.identical?(send_io_ary[i], recv_io_ary[i]))
-          assert(recv_io_ary[i].close_on_exec?)
-        }
+        begin
+          ctls.each {|ctl|
+            next if ctl.level != Socket::SOL_SOCKET || ctl.type != Socket::SCM_RIGHTS
+            recv_io_ary.concat ctl.unix_rights
+          }
+          assert_equal(send_io_ary.length, recv_io_ary.length)
+          send_io_ary.length.times {|i|
+            assert_not_equal(send_io_ary[i].fileno, recv_io_ary[i].fileno)
+            assert(File.identical?(send_io_ary[i], recv_io_ary[i]))
+            assert(recv_io_ary[i].close_on_exec?)
+          }
+        ensure
+          recv_io_ary.each {|io| io.close if !io.closed? }
+        end
       }
     }
   ensure
@@ -248,31 +256,43 @@ class TestSocket_UNIXSocket < Test::Unit::TestCase
     tmpfile = Tempfile.new("s")
     path = tmpfile.path
     tmpfile.close(true)
-    yield klass.new(path), path
+    io = klass.new(path)
+    yield io, path
   ensure
+    io.close
     File.unlink path if path && File.socket?(path)
   end
 
   def test_addr
     bound_unix_socket(UNIXServer) {|serv, path|
-      c = UNIXSocket.new(path)
-      s = serv.accept
-      assert_equal(["AF_UNIX", path], c.peeraddr)
-      assert_equal(["AF_UNIX", ""], c.addr)
-      assert_equal(["AF_UNIX", ""], s.peeraddr)
-      assert_equal(["AF_UNIX", path], s.addr)
-      assert_equal(path, s.path)
-      assert_equal("", c.path)
+      UNIXSocket.open(path) {|c|
+        s = serv.accept
+        begin
+          assert_equal(["AF_UNIX", path], c.peeraddr)
+          assert_equal(["AF_UNIX", ""], c.addr)
+          assert_equal(["AF_UNIX", ""], s.peeraddr)
+          assert_equal(["AF_UNIX", path], s.addr)
+          assert_equal(path, s.path)
+          assert_equal("", c.path)
+        ensure
+          s.close
+        end
+      }
     }
   end
 
   def test_cloexec
     bound_unix_socket(UNIXServer) {|serv, path|
-      c = UNIXSocket.new(path)
-      s = serv.accept
-      assert(serv.close_on_exec?)
-      assert(c.close_on_exec?)
-      assert(s.close_on_exec?)
+      UNIXSocket.open(path) {|c|
+        s = serv.accept
+        begin
+          assert(serv.close_on_exec?)
+          assert(c.close_on_exec?)
+          assert(s.close_on_exec?)
+        ensure
+          s.close
+        end
+      }
     }
   end
 
@@ -393,12 +413,13 @@ class TestSocket_UNIXSocket < Test::Unit::TestCase
   end
 
   def test_epipe # [ruby-dev:34619]
-    s1, s2 = UNIXSocket.pair
-    s1.shutdown(Socket::SHUT_WR)
-    assert_raise(Errno::EPIPE) { s1.write "a" }
-    assert_equal(nil, s2.read(1))
-    s2.write "a"
-    assert_equal("a", s1.read(1))
+    UNIXSocket.pair {|s1, s2|
+      s1.shutdown(Socket::SHUT_WR)
+      assert_raise(Errno::EPIPE) { s1.write "a" }
+      assert_equal(nil, s2.read(1))
+      s2.write "a"
+      assert_equal("a", s1.read(1))
+    }
   end
 
   def test_socket_pair_with_block
@@ -463,15 +484,21 @@ class TestSocket_UNIXSocket < Test::Unit::TestCase
     return if /linux/ !~ RUBY_PLATFORM
     Dir.mktmpdir {|d|
       sockpath = "#{d}/sock"
-      serv = Socket.unix_server_socket(sockpath)
-      Socket.unix(sockpath)
-      s, = serv.accept
-      cred = s.getsockopt(:SOCKET, :PEERCRED)
-      inspect = cred.inspect
-      assert_match(/ pid=#{$$} /, inspect)
-      assert_match(/ euid=#{Process.euid} /, inspect)
-      assert_match(/ egid=#{Process.egid} /, inspect)
-      assert_match(/ \(ucred\)/, inspect)
+      Socket.unix_server_socket(sockpath) {|serv|
+        Socket.unix(sockpath) {|c|
+          s, = serv.accept
+          begin
+            cred = s.getsockopt(:SOCKET, :PEERCRED)
+            inspect = cred.inspect
+            assert_match(/ pid=#{$$} /, inspect)
+            assert_match(/ euid=#{Process.euid} /, inspect)
+            assert_match(/ egid=#{Process.egid} /, inspect)
+            assert_match(/ \(ucred\)/, inspect)
+          ensure
+            s.close
+          end
+        }
+      }
     }
   end
 
@@ -493,18 +520,24 @@ class TestSocket_UNIXSocket < Test::Unit::TestCase
     return if /linux/ !~ RUBY_PLATFORM
     Dir.mktmpdir {|d|
       sockpath = "#{d}/sock"
-      serv = Socket.unix_server_socket(sockpath)
-      c = Socket.unix(sockpath)
-      s, = serv.accept
-      s.setsockopt(:SOCKET, :PASSCRED, 1)
-      c.print "a"
-      msg, _, _, cred = s.recvmsg
-      inspect = cred.inspect
-      assert_equal("a", msg)
-      assert_match(/ pid=#{$$} /, inspect)
-      assert_match(/ uid=#{Process.uid} /, inspect)
-      assert_match(/ gid=#{Process.gid} /, inspect)
-      assert_match(/ \(ucred\)/, inspect)
+      Socket.unix_server_socket(sockpath) {|serv|
+        Socket.unix(sockpath) {|c|
+          s, = serv.accept
+          begin
+            s.setsockopt(:SOCKET, :PASSCRED, 1)
+            c.print "a"
+            msg, _, _, cred = s.recvmsg
+            inspect = cred.inspect
+            assert_equal("a", msg)
+            assert_match(/ pid=#{$$} /, inspect)
+            assert_match(/ uid=#{Process.uid} /, inspect)
+            assert_match(/ gid=#{Process.gid} /, inspect)
+            assert_match(/ \(ucred\)/, inspect)
+          ensure
+            s.close
+          end
+        }
+      }
     }
   end
 
@@ -550,14 +583,18 @@ class TestSocket_UNIXSocket < Test::Unit::TestCase
   def test_getpeereid
     Dir.mktmpdir {|d|
       path = "#{d}/sock"
-      serv = Socket.unix_server_socket(path)
-      c = Socket.unix(path)
-      s, = serv.accept
-      begin
-        assert_equal([Process.euid, Process.egid], c.getpeereid)
-        assert_equal([Process.euid, Process.egid], s.getpeereid)
-      rescue NotImplementedError
-      end
+      Socket.unix_server_socket(path) {|serv|
+        Socket.unix(path) {|c|
+          s, = serv.accept
+          begin
+            assert_equal([Process.euid, Process.egid], c.getpeereid)
+            assert_equal([Process.euid, Process.egid], s.getpeereid)
+          rescue NotImplementedError
+          ensure
+            s.close
+          end
+        }
+      }
     }
   end