Skip to content

Commit

Permalink
merge revision(s) 62671: [Backport #14571]
Browse files Browse the repository at this point in the history
	resolv.rb: close socket

	* lib/resolv.rb (UnconnectedUDP#lazy_initialize): store new
	  sockets before binding, so the sockets get closed when the
	  requester is closing.

	* lib/resolv.rb (ConnectedUDP#lazy_initialize): ditto.

	* lib/resolv.rb (UnconnectedUDP#close): synchronize to get rid of
	  race condition.

	* lib/resolv.rb (ConnectedUDP#close): ditto.

	[ruby-core:85901] [Bug #14571]

	From: quixoten (Devin Christensen) <quixoten@gmail.com>

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_4@62915 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
  • Loading branch information
nagachika committed Mar 25, 2018
1 parent 206a798 commit ad0b27d
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 36 deletions.
102 changes: 69 additions & 33 deletions lib/resolv.rb
Expand Up @@ -735,35 +735,47 @@ class UnconnectedUDP < Requester # :nodoc:
def initialize(*nameserver_port)
super()
@nameserver_port = nameserver_port
@socks_hash = {}
@socks = []
nameserver_port.each {|host, port|
if host.index(':')
bind_host = "::"
af = Socket::AF_INET6
else
bind_host = "0.0.0.0"
af = Socket::AF_INET
end
next if @socks_hash[bind_host]
begin
sock = UDPSocket.new(af)
rescue Errno::EAFNOSUPPORT
next # The kernel doesn't support the address family.
end
sock.do_not_reverse_lookup = true
DNS.bind_random_port(sock, bind_host)
@socks << sock
@socks_hash[bind_host] = sock
@initialized = false
@mutex = Thread::Mutex.new
end

def lazy_initialize
@mutex.synchronize {
next if @initialized
@initialized = true
@socks_hash = {}
@socks = []
@nameserver_port.each {|host, port|
if host.index(':')
bind_host = "::"
af = Socket::AF_INET6
else
bind_host = "0.0.0.0"
af = Socket::AF_INET
end
next if @socks_hash[bind_host]
begin
sock = UDPSocket.new(af)
rescue Errno::EAFNOSUPPORT
next # The kernel doesn't support the address family.
end
@socks << sock
@socks_hash[bind_host] = sock
sock.do_not_reverse_lookup = true
DNS.bind_random_port(sock, bind_host)
}
}
self
end

def recv_reply(readable_socks)
lazy_initialize
reply, from = readable_socks[0].recvfrom(UDPSize)
return reply, [from[3],from[1]]
end

def sender(msg, data, host, port=Port)
lazy_initialize
sock = @socks_hash[host.index(':') ? "::" : "0.0.0.0"]
return nil if !sock
service = [host, port]
Expand All @@ -775,9 +787,14 @@ def sender(msg, data, host, port=Port)
end

def close
super
@senders.each_key {|service, id|
DNS.free_request_id(service[0], service[1], id)
@mutex.synchronize {
if @initialized
super
@senders.each_key {|service, id|
DNS.free_request_id(service[0], service[1], id)
}
@initialized = false
end
}
end

Expand All @@ -801,20 +818,32 @@ def initialize(host, port=Port)
super()
@host = host
@port = port
is_ipv6 = host.index(':')
sock = UDPSocket.new(is_ipv6 ? Socket::AF_INET6 : Socket::AF_INET)
@socks = [sock]
sock.do_not_reverse_lookup = true
DNS.bind_random_port(sock, is_ipv6 ? "::" : "0.0.0.0")
sock.connect(host, port)
@mutex = Thread::Mutex.new
@initialized = false
end

def lazy_initialize
@mutex.synchronize {
next if @initialized
@initialized = true
is_ipv6 = @host.index(':')
sock = UDPSocket.new(is_ipv6 ? Socket::AF_INET6 : Socket::AF_INET)
@socks = [sock]
sock.do_not_reverse_lookup = true
DNS.bind_random_port(sock, is_ipv6 ? "::" : "0.0.0.0")
sock.connect(@host, @port)
}
self
end

def recv_reply(readable_socks)
lazy_initialize
reply = readable_socks[0].recv(UDPSize)
return reply, nil
end

def sender(msg, data, host=@host, port=@port)
lazy_initialize
unless host == @host && port == @port
raise RequestError.new("host/port don't match: #{host}:#{port}")
end
Expand All @@ -825,10 +854,15 @@ def sender(msg, data, host=@host, port=@port)
end

def close
super
@senders.each_key {|from, id|
DNS.free_request_id(@host, @port, id)
}
@mutex.synchronize do
if @initialized
super
@senders.each_key {|from, id|
DNS.free_request_id(@host, @port, id)
}
@initialized = false
end
end
end

class Sender < Requester::Sender # :nodoc:
Expand All @@ -842,6 +876,7 @@ def send

class MDNSOneShot < UnconnectedUDP # :nodoc:
def sender(msg, data, host, port=Port)
lazy_initialize
id = DNS.allocate_request_id(host, port)
request = msg.encode
request[0,2] = [id].pack('n')
Expand All @@ -851,6 +886,7 @@ def sender(msg, data, host, port=Port)
end

def sender_for(addr, msg)
lazy_initialize
@senders[msg.id]
end
end
Expand Down
19 changes: 19 additions & 0 deletions test/resolv/test_dns.rb
Expand Up @@ -3,6 +3,7 @@
require 'resolv'
require 'socket'
require 'tempfile'
require 'minitest/mock'

class TestResolvDNS < Test::Unit::TestCase
def setup
Expand Down Expand Up @@ -221,4 +222,22 @@ def test_too_big_label_address
}
assert_operator(2**14, :<, m.to_s.length)
end

def assert_no_fd_leak
socket = assert_throw(self) do |tag|
Resolv::DNS.stub(:bind_random_port, ->(s, *) {throw(tag, s)}) do
yield.getname("8.8.8.8")
end
end

assert_predicate(socket, :closed?, "file descriptor leaked")
end

def test_no_fd_leak_connected
assert_no_fd_leak {Resolv::DNS.new(nameserver_port: [['127.0.0.1', 53]])}
end

def test_no_fd_leak_unconnected
assert_no_fd_leak {Resolv::DNS.new}
end
end
6 changes: 3 additions & 3 deletions version.h
@@ -1,10 +1,10 @@
#define RUBY_VERSION "2.4.4"
#define RUBY_RELEASE_DATE "2018-03-23"
#define RUBY_PATCHLEVEL 284
#define RUBY_RELEASE_DATE "2018-03-25"
#define RUBY_PATCHLEVEL 285

#define RUBY_RELEASE_YEAR 2018
#define RUBY_RELEASE_MONTH 3
#define RUBY_RELEASE_DAY 23
#define RUBY_RELEASE_DAY 25

#include "ruby/version.h"

Expand Down

0 comments on commit ad0b27d

Please sign in to comment.