Dynamic rescue blocks don't catch Java exceptions #1598

Closed
tvo opened this Issue Mar 30, 2014 · 1 comment

2 participants

@tvo

Hey,

I found that dynamic rescue blocks are unable to catch Java exceptions. The === operator on the module in the rescue clause is simply never called.

This is shown by this example:

module RescuableException
  def self.===(exception)
    puts "HERE: #{exception.inspect}"
    true
  end
end

begin
  raise java.lang.NullPointerException.new
rescue RescuableException => e
  puts e.inspect
end

puts 'RESCUE WORKED!'

With any Ruby exception, === is invoked and the exception is caught. With any Java exception, the program never reaches the rescue block or the puts 'RESCUE WORKED!' line. Instead, it exits with the following call trace, indicating that the exception was never caught:

NativeConstructorAccessorImpl.java:-2:in `newInstance0': java.lang.NullPointerException
  from NativeConstructorAccessorImpl.java:57:in `newInstance'
  from DelegatingConstructorAccessorImpl.java:45:in `newInstance'
  from Constructor.java:526:in `newInstance'
  from JavaConstructor.java:259:in `newInstanceDirect'
  from ConstructorInvoker.java:79:in `call'
  from ConstructorInvoker.java:160:in `call'
  from CachingCallSite.java:316:in `cacheAndCall'
  from CachingCallSite.java:145:in `callBlock'
  from CachingCallSite.java:149:in `call'
  from ConcreteJavaProxy.java:48:in `call'
  from CachingCallSite.java:316:in `cacheAndCall'
  from CachingCallSite.java:145:in `callBlock'
  from CachingCallSite.java:149:in `call'
  from RubyClass.java:788:in `newInstance'
  from RubyClass$INVOKER$i$newInstance.gen:-1:in `call'
  from JavaMethod.java:280:in `call'
  from ConcreteJavaProxy.java:141:in `call'
  from CachingCallSite.java:306:in `cacheAndCall'
  from CachingCallSite.java:136:in `call'
  from CallNoArgNode.java:71:in `interpret'
  from FCallOneArgNode.java:36:in `interpret'
  from NewlineNode.java:105:in `interpret'
  from RescueNode.java:221:in `executeBody'
  from RescueNode.java:116:in `interpret'
  from BeginNode.java:83:in `interpret'
  from NewlineNode.java:105:in `interpret'
  from BlockNode.java:71:in `interpret'
  from RootNode.java:129:in `interpret'
  from ASTInterpreter.java:121:in `INTERPRET_ROOT'
  from Ruby.java:842:in `runInterpreter'
  from Ruby.java:850:in `runInterpreter'
  from Ruby.java:689:in `runNormally'
  from Ruby.java:524:in `runFromMain'
  from Main.java:397:in `doRunFromMain'
  from Main.java:292:in `internalRun'
  from Main.java:219:in `run'
  from Main.java:199:in `main'

I dug around a little in JRuby code and applied the following patch, which does make it work in this particular case:

diff --git a/core/src/main/java/org/jruby/runtime/Helpers.java b/core/src/main/java/org/jruby/runtime/Helpers.java
index 6d4edbc..774e033 100644
--- a/core/src/main/java/org/jruby/runtime/Helpers.java
+++ b/core/src/main/java/org/jruby/runtime/Helpers.java
@@ -1025,7 +1050,7 @@ public class Helpers {
                 // rescue Object needs to catch Java exceptions
                 runtime.getObject() == catchable ||

-                // rescue StandardError needs t= catch Java exceptions
+                // rescue StandardError needs to catch Java exceptions
                 runtime.getStandardError() == catchable) {

             if (throwable instanceof RaiseException) {
@@ -1048,6 +1073,12 @@ public class Helpers {
                     return true;
                 }
             }
+
+        } else if (catchable instanceof RubyModule) {
+            IRubyObject exception = JavaUtil.convertJavaToUsableRubyObject(runtime, throwable);
+            IRubyObject result = invoke(context, catchable, "===", exception);
+            return result.isTrue();
+
         }

         return false;
diff --git a/spec/java_integration/exceptions/rescue_spec.rb b/spec/java_integration/exceptions/rescue_spec.rb
index 17b5c74..54eff40 100644
--- a/spec/java_integration/exceptions/rescue_spec.rb
+++ b/spec/java_integration/exceptions/rescue_spec.rb
@@ -48,6 +48,30 @@ describe "A non-wrapped Java throwable" do

     (obj.go rescue 'foo').should == 'foo'
   end
+
+  it "can be rescued dynamically using Module" do
+    mod = Module.new
+    (class << mod; self; end).instance_eval do
+      define_method(:===) { |exception| true }
+    end
+    begin
+      raise java.lang.NullPointerException.new
+    rescue mod => e
+      e.should be_kind_of(java.lang.NullPointerException)
+    end
+  end
+
+  it "can be rescued dynamically using Class" do
+    cls = Class.new
+    (class << cls; self; end).instance_eval do
+      define_method(:===) { |exception| true }
+    end
+    begin
+      raise java.lang.NullPointerException.new
+    rescue cls => e
+      e.should be_kind_of(java.lang.NullPointerException)
+    end
+  end
 end

 describe "A Ruby-level exception" do

I'm not familiar enough with JRuby code to be able to say that this correct and complete, though, so please double check!

Thanks!

-Tobi

@tvo tvo pushed a commit to tvo/jruby that referenced this issue Apr 1, 2014
Tobi Vollebregt Fix dynamic rescue on Java exceptions (issue #1598) 25949a6
@headius
JRuby Team member

Fixed by #1599.

@headius headius closed this Apr 7, 2014
@headius headius added this to the JRuby 1.7.12 milestone Apr 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment