SSLSocket holds selectors, keys, preventing quick cleanup of resources when dereferenced

Charles Oliver Nutter added a comment - 17/Aug/10 11:35 AM I pushed the selector fix to jruby-ossl in dc45408. Looking at the socket cleanup now.

Charles Oliver Nutter added a comment - 17/Aug/10 11:58 AM Attaching some sample scripts to reproduce this, contributed by Brad Pardee. To generate the cert: openssl genrsa 1024 > host.key chmod 400 host.key openssl req -new -x509 -nodes -sha1 -days 365 -key host.key > host.cert Enter whatever for the cert fields. Here's the server script I'm using: require 'openssl' require 'thread' require 'socket' response = <<EOS HTTP/1.1 200 OK X-Runtime: 4 ETag: "732d4032d83bb8ae165266518d6964be" Cache-Control: private, max-age=0, must-revalidate Content-Type: text/html;charset=utf-8 Content-Length: 6 Date: Mon, 02 Aug 2010 18:20:10 GMT alive EOS context = OpenSSL::SSL::SSLContext.new context.cert = OpenSSL::X509::Certificate.new(File::read('host.cert')) context.key = OpenSSL::PKey::RSA.new(File::read('host.key')) server = TCPServer.new(3001) ssl_server = OpenSSL::SSL::SSLServer.new(server, context) puts "Listening on port 3001" sockfds = [ssl_server] count = 0 while true result = IO.select(sockfds) if result[0] result[0].each do |fd| if fd == ssl_server sockfds << ssl_server.accept else count += 1 begin fd.readpartial(999) # if count % 5 == 0 # fd.close # sockfds.delete(fd) # elsif count % 5 == 3 # fd.write response[0,10] # sleep 1 # fd.close # sockfds.delete(fd) # else fd.write response fd.flush # end rescue fd.close sockfds.delete(fd) end end end end end And here's the client: require 'net/http' require 'net/https' require 'uri' class PersistController @@test_uri = URI.parse('https://localhost:3001/') @@http2 = Net::HTTP.new(@@test_uri.host, @@test_uri.port) @@req2 = Net::HTTP::Get.new @@test_uri.request_uri @@http2.use_ssl = true @@http2.start @@req2.add_field 'Connection', 'keep-alive' @@req2.add_field 'Keep-Alive', 30 def test2 begin response = @@http2.request @@req2 #puts "#{Time.now} received #{response.body.chomp} fds=#{fdcount}" response.body.chomp puts "after request fdcount=#{fdcount}" rescue IOError, EOFError, Timeout::Error, Errno::ECONNABORTED, Errno::ECONNRESET, Errno::EPIPE => e @@http2.finish @@http2.start puts "after reset fdcount=#{fdcount}" retry end end def fdcount return `lsof -p #$$ | grep PIPE | wc -l`.chomp path = "/proc/#$$/fd" return `ls -1 #{path} | wc -l`.chomp if File.exist?(path) return '0' end end loop do PersistController.new.test2 gets end The client will perform repeated https requests to the server, separated by a "gets" call. In between requests, if you drop and restart the server, you'll see that on most systems the pipe/descriptor count increases (note that there's a few ways to get the pipe/descriptor count, so fdcount will need to be tweaked for different platforms). The problem was that when the socket fails, http.rb tries to close it and reopen a new one. But because there's data waiting and the SSLSocket instance is not "forced" to close, it doesn't actually shutdown the socket or the selectors. My fix mentioned about eliminates the use of excessive selectors, and a fix I've committed in 81de17e forces a full SSLSocket shutdown when sysclose is called. With these changes, the fd count remains straight and level, regardless of server bounces: ~/projects/jruby/ssl_fd_leak &#10132; jruby persist_controller.rb warning: peer certificate won't be verified in this SSL session after request fdcount= 3 after reset fdcount= 3 after request fdcount= 3 after reset fdcount= 3 after request fdcount= 3 I'm not sure how to write a test for this, since you'd have to be able to inspect the finalizer queue to ensure no lingering sockets were there. Perhaps a test that sysclose really does shut down the socket, even if there's data waiting?

Hiro Asari added a comment - 17/Aug/10 12:05 PM I wonder if this is related to JRUBY-5015 .

Charles Oliver Nutter added a comment - 17/Aug/10 12:05 PM Fixed for 0.7.2 as described above. I've attached a 0.7.2.dev gem to this issue for users that need the fix now.

Hiroshi Nakamura added a comment - 18/Aug/10 2:59 AM For the commit 81de17e, it's me who didn't changed it to 'forceClose()' in 869737a0. http://github.com/jruby/jruby-ossl/commit/869737a0 As I wrote in the commit log, we should 'force close' outbound channel even if we have remaining data to be sent. I fixed other 'close's but I slipped to change sysclose. I should have fixed sysclose, too.

Hiroshi Nakamura added a comment - 18/Aug/10 4:30 AM And I confirmed the commit dc45408 fixes the problem and it's correct. (Of course!) We may remove useless selectedKey iteration in waitSelect().

Hiroshi Nakamura added a comment - 18/Aug/10 6:08 AM Applied 2 additional fixes. a3422063ffeb0f8ca789 Additional fix for JRUBY-5018 : delete unnecesary selectedKeys iteration cf5856ab71e5b6566b9a Additional fix for JRUBY-5018 : no need to try to do shutdown for server socket.