Fix for JRUBY-3817: Timeouts in Net::HTTP raise Timeout::ExitExceptio…

…n instead of Timeout::Error for Ruby 1.8

Fix for JRUBY-3820: timeout library should throw an anonymous exception type while unrolling stack

src/org/jruby/ext/Timeout.java

@@ -18,6 +18,7 @@
 import org.jruby.javasupport.util.RuntimeHelpers;
 import org.jruby.runtime.Arity;
 import org.jruby.runtime.Block;
+import org.jruby.runtime.ObjectAllocator;
 import org.jruby.runtime.ThreadContext;
 import org.jruby.runtime.builtin.IRubyObject;
 import org.jruby.runtime.load.Library;
@@ -29,6 +30,12 @@ public void load(Ruby runtime, boolean wrap) throws IOException {
         RubyClass timeoutError = runtime.defineClassUnder("Error", runtime.getInterrupt(), runtime.getInterrupt().getAllocator(), timeout);
         runtime.defineClassUnder("ExitException", runtime.getException(), runtime.getInterrupt().getAllocator(), timeout);
 
+        // Here we create an "anonymous" exception type used for unrolling the stack.
+        // MRI creates a new one for *every call* to timeout, which can be costly.
+        // We opt to use a single exception type for all cases to avoid this overhead.
+        RubyClass anonEx = runtime.defineClassUnder("AnonymousException", runtime.getException(), runtime.getInterrupt().getAllocator(), timeout);
+        anonEx.setBaseName(null); // clear basename so it's anonymous when raising
+
         // These are not really used by timeout, but exposed for compatibility
         timeout.defineConstant("THIS_FILE", RubyRegexp.newRegexp(runtime, "timeout\\.rb", 0));
         timeout.defineConstant("CALLER_OFFSET", RubyFixnum.newFixnum(runtime, 0));
@@ -80,18 +87,20 @@ public static IRubyObject timeout(final ThreadContext context, IRubyObject timeo
         Future timeoutFuture = null;
 
         try {
-            timeoutFuture = timeoutExecutor.schedule(timeoutRunnable, 
-                    (long)(seconds.convertToFloat().getDoubleValue() * 1000000), TimeUnit.MICROSECONDS);
+            try {
+                timeoutFuture = timeoutExecutor.schedule(timeoutRunnable,
+                        (long)(seconds.convertToFloat().getDoubleValue() * 1000000), TimeUnit.MICROSECONDS);
 
-            return block.yield(context, seconds);
+                return block.yield(context, seconds);
+            } finally {
+                killTimeoutThread(context, timeoutFuture);
+            }
         } catch (RaiseException re) {
-            if (re.getException().getMetaClass() == runtime.getClassFromPath("Timeout::ExitException")) {
+            if (re.getException().getMetaClass() == runtime.getClassFromPath("Timeout::AnonymousException")) {
                 return raiseTimeoutError(context, re);
             } else {
                 throw re;
             }
-        } finally {
-            killTimeoutThread(context, timeoutFuture);
         }
     }
 
@@ -109,17 +118,21 @@ public static IRubyObject timeout(final ThreadContext context, IRubyObject timeo
             return raiseBecauseCritical(context);
         }
 
-        final IRubyObject exception = exceptionType.isNil() ? runtime.getClassFromPath("Timeout::ExitException") : exceptionType;
+        final IRubyObject exception = exceptionType.isNil() ? runtime.getClassFromPath("Timeout::AnonymousException") : exceptionType;
         final RubyThread currentThread = context.getThread();
 
         Runnable timeoutRunnable = prepareRunnableWithException(currentThread, exception, runtime);
         Future timeoutFuture = null;
 
         try {
-            timeoutFuture = timeoutExecutor.schedule(timeoutRunnable, 
-                    (long)(seconds.convertToFloat().getDoubleValue() * 1000000), TimeUnit.MICROSECONDS);
+            try {
+                timeoutFuture = timeoutExecutor.schedule(timeoutRunnable,
+                        (long)(seconds.convertToFloat().getDoubleValue() * 1000000), TimeUnit.MICROSECONDS);
 
-            return block.yield(context, seconds);
+                return block.yield(context, seconds);
+            } finally {
+                killTimeoutThread(context, timeoutFuture);
+            }
         } catch (RaiseException re) {
             // if it's the exception we're expecting
             if (re.getException().getMetaClass() == exception) {
@@ -131,15 +144,13 @@ public static IRubyObject timeout(final ThreadContext context, IRubyObject timeo
 
             // otherwise, rethrow
             throw re;
-        } finally {
-            killTimeoutThread(context, timeoutFuture);
         }
     }
 
     private static Runnable prepareRunnable(final RubyThread currentThread, final Ruby runtime) {
         Runnable timeoutRunnable = new Runnable() {
             public void run() {
-                raiseInThread(runtime, currentThread, runtime.getClassFromPath("Timeout::ExitException"));
+                raiseInThread(runtime, currentThread, runtime.getClassFromPath("Timeout::AnonymousException"));
             }
         };
         return timeoutRunnable;

test/test_timeout.rb

@@ -1,6 +1,7 @@
 require 'test/unit'
 require 'timeout'
 require 'socket'
+require 'net/http'
 
 class TestTimeout < Test::Unit::TestCase
   def test_timeout_for_loop
@@ -50,8 +51,39 @@ def test_timeout_sysread_socket
     rescue Timeout::Error
       ok = true
     end
+
+    assert ok, "IO#sysread was not interrupted by timeout"
   ensure
     begin; server.close; rescue Exception; end
     begin; client.close; rescue Exception; end
   end
+
+  def foo
+    sleep 5
+  rescue Exception => e
+    @in_foo = e
+    raise e
+  end
+
+  # JRUBY-3817
+  def test_net_http_timeout
+    assert_raises Timeout::Error do
+      http = Net::HTTP.new('www.google.de')
+      http.open_timeout = 0.01
+      response = http.start do |h|
+        h.request_get '/index.html'
+      end
+    end
+  end
+
+  def test_timeout_raises_anon_class_to_unroll
+    begin
+      timeout(0.1) { foo }
+    rescue Timeout::Error
+      ok = true
+    end
+
+    assert ok, "Timeout::Error was not eventually delivered to caller"
+    assert @in_foo.class.name == "", "Non-anonymous exception type raised in intervening stack"
+  end
 end