Ruby exceptions passing through Java Integration and back remain wrapped

[headius]

This is either a problem with JI (my area) or IR (@enebo or @subbuss).

It appears that exceptions raised by Ruby code called by Java code called through Java integration (a Ruby exception that passes through a JI call on its way out) remain wrapped and are presented to rescue blocks as the wrapped RaiseException rather than the contained Ruby exception.

 $ jruby -J-classpath test/target/test-classes/ -S rspec spec/java_integration/exceptions/rescue_spec.rb 
.................FF

Failures:

  1) A ruby exception raised through java and back to ruby its class and message is preserved
     Failure/Error: e.message.should == "it comes from ruby"
       expected: "it comes from ruby"
            got: "org.jruby.exceptions.RaiseException: (RuntimeError) it comes from ruby" (using ==)
     # ./spec/java_integration/exceptions/rescue_spec.rb:208:in `(root)'

  2) A ruby exception raised through java and back to ruby via a different thread its class and message is preserved
     Failure/Error: e.message.should == "it comes from ruby"
       expected: "it comes from ruby"
            got: "org.jruby.exceptions.RaiseException: (RuntimeError) it comes from ruby" (using ==)
     # ./spec/java_integration/exceptions/rescue_spec.rb:222:in `(root)'

This happens regardless of JIT status, and since I have not done anything with JI and exceptions recently I suspect this is a flaw in how IR handles unwrapping exceptions for rescue.

[subbuss]

I think it is because the interpreter (in this debug output below) receives a NativeException and it only unwraps RaiseExceptions. This is debug output from a short test snippet to reproduce that error. We should chat about whether NativeExceptions receive special handling or if this is something that requires changing in JI ...

...
2014-07-20T22:59:46.576-05:00: Interpreter: I: %v_1 = search_const(ExceptionRunner, scope<x.rb>, no-private-consts=false)
2014-07-20T22:59:46.576-05:00: Interpreter: I: %v_2 = call_0o(NORMAL, 'new', %v_1, []){0O}
2014-07-20T22:59:46.576-05:00: Interpreter: I: %v_1 = call(NORMAL, 'do_it_now', %v_2, [], &%self:CLOSURE x.rb_CLOSURE_1[x.rb:5])
2014-07-20T22:59:46.585-05:00: Interpreter: I: thread_poll
2014-07-20T22:59:46.585-05:00: Interpreter: I: line_num(6)
2014-07-20T22:59:46.585-05:00: Interpreter: I: %cl_1_0 = call_1o(FUNCTIONAL, 'raise', %self, ["it comes from ruby"]){1O}
Exception `RuntimeError' at java/lang/Thread.java:1567 - it comes from ruby
2014-07-20T22:59:46.589-05:00: Interpreter: in scope: CLOSURE x.rb_CLOSURE_1[x.rb:5], caught Java throwable: org.jruby.exceptions.RaiseException: (RuntimeError) it comes from ruby; excepting instr: %cl_1_0 = call_1o(FUNCTIONAL, 'raise', %self, ["it comes from ruby"]){1O}
2014-07-20T22:59:46.589-05:00: Interpreter: ipc for rescuer: -1
2014-07-20T22:59:46.591-05:00: Interpreter: in scope: Script: file: x.rbSCRIPT_BODY x.rb[x.rb:0], caught Java throwable: org.jruby.exceptions.RaiseException: (RuntimeError) it comes from ruby; excepting instr: %v_1 = call(NORMAL, 'do_it_now', %v_2, [], &%self:CLOSURE x.rb_CLOSURE_1[x.rb:5])
2014-07-20T22:59:46.591-05:00: Interpreter: ipc for rescuer: 24
2014-07-20T22:59:46.591-05:00: Interpreter: I: %v_1 = recv_ruby_exc
2014-07-20T22:59:46.591-05:00: Interpreter: I: %v_3 = search_const(RuntimeError, scope<x.rb>, no-private-consts=false)
2014-07-20T22:59:46.591-05:00: Interpreter: I: %v_4 = rescue_eqq(Array:[%v_3], %v_1)
2014-07-20T22:59:46.591-05:00: Interpreter: I: b_true(%v_4, LBL_4:29)
2014-07-20T22:59:46.591-05:00: Interpreter: I: %t_e_5 = get_global_var($!)
2014-07-20T22:59:46.591-05:00: Interpreter: I: line_num(9)
2014-07-20T22:59:46.591-05:00: Interpreter: I: store_lvar(%t_e_5, x.rb, e(0:0))
2014-07-20T22:59:46.591-05:00: Interpreter: I: %v_1 = call_1o(FUNCTIONAL, 'p', %self, [%t_e_5]){1O}
#<NativeException: org.jruby.exceptions.RaiseException: (RuntimeError) it comes from ruby>
....

[headius]

NativeException should almost never happen. We got rid of those in 1.6 I think. We need to figure out who's creating this NativeException and fix it on that end, I suppose.

[subbuss]

Oh, it is gone? Then IRRuntimeHelpers has stale code :) ... See https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java#L297 ... and on line 304, it sets $! to that.

[headius]

Ok, sorry, it's not completely gone. For backward compatibility, if someone rescues NativeException and we receive a Java exception, then we wrap it before we give it to them. But otherwise we should be propagating the exception as-is and not wrapping it.

[subbuss]

There seems to be couple things going on.

(1) JI code somewhere is setting $! to a NativeException obj, but the exception obj that enters the interpreter is actually a RaiseException. Since the normal ruby case doesn't set $!, the NativeException obj gets passed along. Whatever passes the correct RaiseException obj should also be setting $! to the same object rather than set a NativeException obj. For the test snippet in question, this is the source of the bug.

(2) In addition, in IRRuntimeHelpers code above, in some scenarios, looks like $! is being updated to a different object type for the JI case. I don't fully understand why this is required and also what the correct wrapping/unwrapping logic is for that. Looking at Helpers.checkJavaException, I see a lot of logic there and it is unwrapped/wrapped differently depending on exception type in the rescue clause .. so, I would think something similar would happen in order to set "$!"

This is probably best fixed by one of you since you understand JI and I haven't delved into that part of JRuby yet, but happy to chat on irc/im about any of this.

[subbuss]

An update on what is going on based on some debugging.

https://gist.githubusercontent.com/subbuss/d29d501f0cadd8b92a39/raw/993c2a9589bec27b57a192542578bf6b7986e92f/gistfile1.txt shows you what is going on.

When the raise in the ruby code executes, $! is correctly set to the RaiseException object. But, when the exception propagates into the JI code, JI catches the exception and does a stack-trace rewrite before rethrowing it. It uses Helpers.rewriteStackTraceAndThrow for this. Here is the code for that method:

public static IRubyObject rewriteStackTraceAndThrow(Throwable t, Ruby runtime) {
    RaiseException rewriteStackTrace = RaiseException.createNativeRaiseException(runtime, t);
    t.setStackTrace(rewriteStackTrace.getStackTrace());
    throwException(t);

    // not reached
    return null;
}

So, this creates a nativeException -- but RaiseException code unconditionally sets $! - which means $! is now set to be the NativeException. However, the helper code is creating this RaiseException simply to generate a stack trace and discards it before rethrowing the original RubyException. But, in the bargain, it has clobbered $!. That, I think is the bug.

[subbuss]

Isn't this fixed now?

[headius]

This has to be fixed. We'd see a lot more issues with our ruby kernel or the JI specs if it wasn't.