SelectorPool file descriptor leak

Have you tried running the exact same test except not calling Net::HTTP.get in your "/test" handler? I'm wondering if this isn't a problem with Net::HTTP but just every request to the server leaving an open file and the current code makes the problem worse because each request spawns another request to the same server.

The only reason I suggest this is I know jruby-rack and TorqueBox have tracked down similar bugs in the past with each HTTP request leaving open files.

Yeah, I tried. Running:

ab -n 100 -c 10 http://127.0.0.1:9090/hello

doesn't create any new pipes.

I think that the pipes are for SelectorPool for each Ruby runtime (org.jruby.Ruby). Selector is needed for each Thread in JRuby and SelectorPool pools those Selectors. When you run 10 threads at the same time, JRuby needs 10 Selectors (20 pipes).

And when I touch REVISION, a new Ruby runtime is generated but older one is not GCed.

Just looking at Kirk's source, ScriptingContainer#destroy() is not called. Would you please change Kirk to call ScriptingContainer#destroy() like jruby-rack and try again? I don't have any evidence that it's the cause of this leak but increasing number of Ruby runtimes should be fixed anyway.

Though Trinidad is using jruby-rack as far as I know so...

That's right - each thread uses two pipes. The problem is that these pipes are not closed when thread is interrupted. It leads to "Too many open files" exception after some time. For now I have extended allowed number of opened files, but it's not a long term solution.

I've checked Kirk's source and it calls ScriptingContainer#terminate() method, as jruby-rack used to do. They dropped using ScriptingContainer some time ago, but I don't know why:
https://github.com/nicksieger/jruby-rack/commit/385ae17d7c860c5151742e1d72b80d7e40771744

Thanks. I confirmed that Kirk looks to call terminate() properly. I'll investigate more.

This patch closes all Selector in SelectorPool at Ruby#tearDown().

diff --git a/src/org/jruby/Ruby.java b/src/org/jruby/Ruby.java
index 38be3f7..b7d7a2a 100644
--- a/src/org/jruby/Ruby.java
+++ b/src/org/jruby/Ruby.java
@@ -2780,6 +2780,8 @@ public final class Ruby {
         if (getJRubyClassLoader() != null) {
             getJRubyClassLoader().tearDown(isDebug());
         }
+        
+        getSelectorPool().terminate();
 
         if (config.isProfilingEntireRun()) {
             System.err.println("\nmain thread profile results:");
diff --git a/src/org/jruby/util/io/SelectorPool.java b/src/org/jruby/util/io/SelectorPool.java
index daa2d0e..6f11526 100644
--- a/src/org/jruby/util/io/SelectorPool.java
+++ b/src/org/jruby/util/io/SelectorPool.java
@@ -76,6 +76,17 @@ public class SelectorPool {
     public synchronized void put(Selector selector) {
         returnToPool(selector);
     }
+    
+    public synchronized void terminate() {
+        while (!pool.isEmpty()) {
+            Reference<Selector> ref = pool.remove(0);
+            try {
+                ref.get().close();
+            } catch (IOException ioe) {
+                // ignore IOException at termination.
+            }
+        }
+    }
 
     private Selector retrieveFromPool() throws IOException {
         // scrub pool

This is a sample script to reproduce. Using Socket (which acquires a Selector from SelectorPool) is a key.

require 'java'
java_import 'org.jruby.embed.ScriptingContainer'
java_import 'org.jruby.embed.LocalContextScope'

10.times do |idx|
  vm = ScriptingContainer.new(LocalContextScope::SINGLETHREAD)
  vm.run_scriptlet(<<-END)
    require 'socket'
    socket = TCPSocket.new('localhost', 80)
    socket << "GET / HTTP/1.0\r\n\r\n"
    socket.read
    socket.close
  END
  vm.terminate
  puts "invoked #{idx + 1} times"
  gets # => check lsof and number of FIFO pipes
end

Here's my understanding.

• JRuby's SelectorPool keeps java.nio.channels.Selectors in soft-referenced pool: https://github.com/jruby/jruby/commit/19fa9537
• Linux JDK implementaions uses 2 fds per 1 Selector object. I don't know how it's implemented on other OSes.
• Those fds are only closed by 'close()' method. Selector does not have finalize() so fds are not closed by GC. (It's my guess based on JDK documentations.) On either OpenJDK, J9 and JRockit.

If I'm correct, we should not use soft-reference for Selector in SelectorPool.

JRuby devs: Can I replace it with a simple direct-reference Selector pool? Is there any concern, especially on Windows?

Good find; soft referencing them and letting them disappear does appear to be the wrong way to handle this. If your hunch is true, the selectors dereference, GC, and stay open since they do not have finalization attached to them.

There's a couple options:

• Hard reference the selectors but add a finalize to SelectorPool to clean them up. Calling terminate on the runtime would then scrub the pool, closing selectors.
• Abstract a wrapper around Selector that we use in JRuby when accessing SelectorPool. Have a finalize on it that can clean up the contained Selector when it GCs.

I think the hard referencing might be the most logical option here, since finalization is never a reliable operation on the JVM. At worst we'd have as many selectors alive as there are concurrent select operations in flight, which is unlikely to hit FD limits in a given process.

I'll give this more thought and come back with an answer...

Thanks Charles. Meanwhile, I fixed master at [78dc4c19ae69f31bbb3c4e4b36048e85aa199d51] based on 'hard-reference' way.

JRUBY-5786: Fix SelectorPool file descriptor leak

Close all selectors in SelectorPool explicitly at Ruby runtime termination.

We need to call java.nio.channel.Selector#close() for freeing internal
resource(s). For example, Selector implementation of Hotspot JVM for Linux
uses 2 file descriptors(fd) per 1 instance and close() is the only way to
release these fds.

We kept soft-referenced selectors in a pool and let GC collect it. Since
Selector does not have a finalizer 2 fds per each selector leaked after
Ruby runtime was terminated, and creating many Ruby runtimes eventually
caused 'Too many open files' error.

Number of selectors in SelectorPool (== number of leaked fds after a runtime
terminated) depends on numbers of maximum *concurrent* select operations
in the runtime. It should not grow so much but we might need to release
spare selectors while running in the future.

I'm going to cherry-pick this fix to jruby-1_6, OK?

Yes, go ahead. I think the number of selectors will rarely be a problem, and if it is we'll cross that bridge when we come to it (by capping the number of selectors or something else).

Cherry-picked at [f6bcb5659e217b5c300eb99db4ebd9f2ad73c535]