jruby 9.3.x: stacktrace is extended on each raise

[AlBundy33]

This issue seems to be introduced in jruby 9.3.0.0 and still exists in latest 9.3.4.0.
9.2.20.1 seems not to be affected.

If you call a script multiple times that raises an error the stacktrace gets bigger and bigger.
for example call a script raise 'foo' in java, catch the error and then call another script raise 'bar' (or the same as before).
In jruby 9.2.x the second stacktrace looks like this

...
Caused by: org.jruby.embed.EvalFailedException: (RuntimeError) bar
	at org.jruby.embed.internal.EmbedEvalUnitImpl.run(EmbedEvalUnitImpl.java:131)
	at org.jruby.embed.jsr223.JRubyEngine.eval(JRubyEngine.java:90)
	... 26 more
Caused by: org.jruby.exceptions.RuntimeError: (RuntimeError) bar
	at RUBY.<main>(<script>:1)

in jruby 9.3.x the second stacktrace looks like this

...
Caused by: org.jruby.exceptions.RuntimeError: (RuntimeError) bar
	at RUBY.<main>(<script>:1)
Caused by: org.jruby.exceptions.RuntimeError: (RuntimeError) foo
	... 1 more

Here is a simple test-case

  @Test
  public void testRubyRaise()
    throws Exception
  {
    try
    {
      final ScriptEngine _jruby1 = new JRubyEngineFactory().getScriptEngine();
      _jruby1.eval("raise 'foo'");
    }
    catch (Exception _e)
    {
      _e.printStackTrace(System.err);
      try (ByteArrayOutputStream _bos = new ByteArrayOutputStream();
          PrintStream _s = new PrintStream(_bos))
      {
        _e.printStackTrace(_s);
        Assert.assertFalse(_bos.toString().contains("bar"), "stack contains bar: " + _bos.toString());
      }
    }
    try
    {
      final ScriptEngine _jruby2 = new JRubyEngineFactory().getScriptEngine();
      _jruby2.eval("raise 'bar'");
    }
    catch (Exception _e)
    {
      _e.printStackTrace(System.err);
      try (ByteArrayOutputStream _bos = new ByteArrayOutputStream();
          PrintStream _s = new PrintStream(_bos))
      {
        _e.printStackTrace(_s);
        Assert.assertFalse(_bos.toString().contains("foo"), "stack contains foo: " + _bos.toString());
      }
    }
  }

tested with jruby.jar from
https://repo1.maven.org/maven2/org/jruby/jruby-dist/9.3.4.0/jruby-dist-9.3.4.0-bin.zip
https://repo1.maven.org/maven2/org/jruby/jruby-dist/9.2.20.1/jruby-dist-9.2.20.1-bin.zip

[headius]

Nice find!

[headius]

Yup that is pretty weird!

[headius]

Something appears to be amiss with how we handle assigning the cause, and it keeps stacking them up. Cause support was added in 9.2.

[headius]

Ok so this is not really a problem with cause logic, it is due to the fact that the current thread's metadata on the Ruby side still shows that there's an active exception in flight, and by default in Ruby 2.5+ when a new exception is raised when an existing exception is in flight that in-flight exception is set as the cause of the new exception. In this case, where it raises repeatedly without any intervening code, the in-flight exception never gets reset and they stack up.

This would eventually be a memory leak if the same script engine invocation kept repeatedly raising a new exception, since they would endlessly stack up.

A simple fix would be to clear the in-flight exception (in the current thread's ThreadContext) upon it being bubbled out of the scripting engine (or container, which I have not tested yet). However, this would break the ability to go back and query the current in-flight exception, which is used at minimum in the tests and potentially in user code that expects the magic $! variable to be set.

A workaround for you would be to clear this value yourself, probably by evaluating code like $! = nil on the same thread.

This is a question of how far out from Ruby execution the in-flight exception should remain in scope. I am generally in favor of having it cleared once it has been raised out of the engine into the Java caller.

[headius]

I've pushed two possible fixes plus a test based on @AlBundy33's in #7203. Surprisingly, it does not fail any of the test suites I expected it to fail.

Ping @enebo @kares for thoughts.

[enebo]

@headius so prior to 9.2 this was how this worked right? We did not have a cause. If so then it seems we are largely just reverting back to original behavior.

[kares]

believe this is an IR issue - as identified above the $! is not reset (and existed in 9.2 as well).
the underlying cause will be the same as in #6972

[headius]

@enebo That is a good point, but I was more worried about clearing $! when it might be queried from Java, which has been the case for some time. However @kares points out that we may be setting it unnecessarily.

@kares If that is the case then we should try to pare this back to just set it when entering a handler, but I feel like that misses cases. For example, I believe we have code where we expect it to also be set even if it does not enter a Ruby rescue block, so we can query it from Java based impls of Ruby core methods.

[enebo]

@headius @kares yeah if we had this problem before then we maybe did a merge where it did not come along to 9.3.

[enebo]

@headius @kares I do not see us investigating and fixing the IR by next week. Should we roll with #7203?