Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AsciidoctorJ fails on JRuby 9.2.0.0 #5204

Closed
robertpanzer opened this Issue May 31, 2018 · 15 comments

Comments

Projects
None yet
4 participants
@robertpanzer
Copy link

commented May 31, 2018

Environment

JRuby 9.2.0.0
OS X: Darwin Roberts-MacBook-Air.local 17.5.0 Darwin Kernel Version 17.5.0: Fri Apr 13 19:32:32 PDT 2018; root:xnu-4570.51.2~1/RELEASE_X86_64 x86_64

Expected Behavior

I tried upgrading the dependency in AsciidoctorJ to 9.2.0.0 and expected the build to run.

Actual Behavior

As soon as the Asciidoctor instance is created, it fails with a java.lang.VerifyError:

java.lang.VerifyError: Bad return type
Exception Details:
  Location:
    org/jruby/gen/InterfaceImpl659984099.converters()Lorg/jruby/RubyArray; @45: areturn
  Reason:
    Type 'org/jruby/runtime/builtin/IRubyObject' (current frame, stack[0]) is not assignable to 'org/jruby/RubyArray' (from method signature)
  Current Frame:
    bci: @45
    flags: { }
    locals: { 'org/jruby/gen/InterfaceImpl659984099', 'org/jruby/runtime/builtin/IRubyObject', 'org/jruby/Ruby' }
    stack: { 'org/jruby/runtime/builtin/IRubyObject' }
  Bytecode:
    0000000: 2ab4 0015 4c2b b900 1d01 004d b200 1f2b
    0000010: 126e 126f b600 272c b600 2d2b 2bb9 0031
    0000020: 0100 126f b200 73b2 003d b600 43b0     

	at java.base/java.lang.Class.getDeclaredConstructors0(Native Method)
	at java.base/java.lang.Class.privateGetDeclaredConstructors(Class.java:3090)
	at java.base/java.lang.Class.getConstructor0(Class.java:3295)
	at java.base/java.lang.Class.getConstructor(Class.java:2110)
	at org.jruby.javasupport.Java.newInterfaceImpl(Java.java:1352)
	at org.jruby.java.proxies.JavaInterfaceTemplate.newInterfaceProxy(JavaInterfaceTemplate.java:285)
	at org.jruby.java.proxies.JavaInterfaceTemplate.access$100(JavaInterfaceTemplate.java:34)
	at org.jruby.java.proxies.JavaInterfaceTemplate$InterfaceProxyFactory.call(JavaInterfaceTemplate.java:233)
	at org.jruby.internal.runtime.methods.DynamicMethod.call(DynamicMethod.java:194)
	at org.jruby.RubyClass.finvoke(RubyClass.java:560)
	at org.jruby.runtime.Helpers.invoke(Helpers.java:353)
	at org.jruby.javasupport.JavaUtil.convertProcToInterface(JavaUtil.java:278)
	at org.jruby.RubyBasicObject.defaultToJava(RubyBasicObject.java:898)
	at org.jruby.RubyBasicObject.toJava(RubyBasicObject.java:876)
	at org.jruby.javasupport.JavaEmbedUtils.rubyToJava(JavaEmbedUtils.java:247)
	at org.asciidoctor.internal.RubyUtils.rubyToJava(RubyUtils.java:19)
	at org.asciidoctor.internal.JRubyAsciidoctorModuleFactory.createAsciidoctorModule(JRubyAsciidoctorModuleFactory.java:27)
...

At the moment the "glue" between the Java API and the Asciidoctor Ruby implementation is moved to JRuby, by having a Java interface AsciidoctorModule [1] and a Ruby class AsciidoctorModule[2] and letting JRuby implement this interface for this Ruby class [3].
It looks like this fails now exactly where JRuby creates the proxy for this interface.

I created a test repository at https://github.com/robertpanzer/jruby92test where I also tried to reproduce the error with a simplified version, but there it succeeds.
The repo also contains a test with Asciidoctor though, and that one fails.

Maybe you have an idea if we did sth wrong in AsciidoctorJ or if this is a real bug.
For the future I am considering to remove the dynamic implementation of this interface anyway, so that I have a workaround (asciidoctor/asciidoctorj#652), but it would still be nice to know to cause of this error.

[1] https://github.com/asciidoctor/asciidoctorj/blob/master/asciidoctorj-core/src/main/java/org/asciidoctor/internal/AsciidoctorModule.java
[2] https://github.com/asciidoctor/asciidoctorj/blob/master/asciidoctorj-core/src/main/resources/org/asciidoctor/internal/asciidoctorclass.rb
[3] https://github.com/asciidoctor/asciidoctorj/blob/master/asciidoctorj-core/src/main/java/org/asciidoctor/internal/JRubyAsciidoctorModuleFactory.java#L27

@headius

This comment has been minimized.

Copy link
Member

commented May 31, 2018

Nice! @kares is this changes you made or I made? I'm at the pub trying not to crash from jet lag 😀

@robertpanzer This shouldn't he a difficult fix but probably no workaround.

@robertpanzer

This comment has been minimized.

Copy link
Author

commented May 31, 2018

Cheers! :) 🍻

@kares

This comment has been minimized.

Copy link
Member

commented May 31, 2018

@headius did not touch this part much ... no clues. @robertpanzer we might need your Java version.

@robertpanzer

This comment has been minimized.

Copy link
Author

commented May 31, 2018

I am seeing this error on both Java 8 and 10 identifying like this:

java version "10.0.1" 2018-04-17
Java(TM) SE Runtime Environment 18.3 (build 10.0.1+10)
Java HotSpot(TM) 64-Bit Server VM 18.3 (build 10.0.1+10, mixed mode)
java version "1.8.0_131"
Java(TM) SE Runtime Environment (build 1.8.0_131-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.131-b11, mixed mode)
@kares

This comment has been minimized.

Copy link
Member

commented Jun 6, 2018

so I looked into the presented case - its quite "edgy" :
implementing a Java interface with org.jruby types in it, than having a .rb impl loaded ...
hard to follow all the pieces - so it would be best to have a reproducer, I am sure we can get it resolved if it was working previously. also managed to fix some VerifyErrors on master recently but I believe this might be smt different.

@robertpanzer

This comment has been minimized.

Copy link
Author

commented Jun 8, 2018

Yeah, I know, we went to the limits :-)
Unfortunately I wasn't able to create a minimal reproducer, I can try again next week.
The project I created is too big I guess?

@headius

This comment has been minimized.

Copy link
Member

commented Jun 11, 2018

There's likely a simple tweak here, to always cast the value to be returned to the return type of the method we're implementing. Doing a plain checkcast will obviously raise a ClassCastException within the generated interface impl if the type doesn't match...is that ok?

@headius

This comment has been minimized.

Copy link
Member

commented Jun 11, 2018

I can reproduce with this simple case:

ReturnsArray.java

import org.jruby.RubyArray;

public interface ReturnsArray {
  public RubyArray doIt();
}
$ jruby -e 'class Foo; include Java::ReturnsArray; def doIt; [1]; end; end; Foo.new.doIt'
	Unhandled Java exception: java.lang.VerifyError: Bad return type
Exception Details:
  Location:
    Foo_436129788.doIt()Lorg/jruby/RubyArray; @42: areturn
  Reason:
    Type 'org/jruby/runtime/builtin/IRubyObject' (current frame, stack[0]) is not assignable to 'org/jruby/RubyArray' (from method signature)
  Current Frame:
    bci: @42
    flags: { }
    locals: { 'Foo_436129788', top, 'org/jruby/Ruby' }
    stack: { 'org/jruby/runtime/builtin/IRubyObject' }
  Bytecode:
    0x0000000: 2ab9 0019 0100 4db2 001b 2a12 1c12 1d12
    0x0000010: 1fb6 0025 2cb6 002b 2a2a b900 2f01 0012
    0x0000020: 1db2 0033 b200 39b6 003f b0            

java.lang.VerifyError: Bad return type
Exception Details:
  Location:
    Foo_436129788.doIt()Lorg/jruby/RubyArray; @42: areturn
  Reason:
    Type 'org/jruby/runtime/builtin/IRubyObject' (current frame, stack[0]) is not assignable to 'org/jruby/RubyArray' (from method signature)
  Current Frame:
    bci: @42
    flags: { }
    locals: { 'Foo_436129788', top, 'org/jruby/Ruby' }
    stack: { 'org/jruby/runtime/builtin/IRubyObject' }
  Bytecode:
    0x0000000: 2ab9 0019 0100 4db2 001b 2a12 1c12 1d12
    0x0000010: 1fb6 0025 2cb6 002b 2a2a b900 2f01 0012
    0x0000020: 1db2 0033 b200 39b6 003f b0            

        getDeclaredConstructors0 at java/lang/Class.java:-2
  privateGetDeclaredConstructors at java/lang/Class.java:2671
                 getConstructor0 at java/lang/Class.java:3075
                  getConstructor at java/lang/Class.java:1825
           setRubyClassAllocator at org/jruby/RubyClass.java:175
               generateRealClass at org/jruby/javasupport/Java.java:1510

And I can fix it with the following patch:

diff --git a/core/src/main/java/org/jruby/java/codegen/RealClassGenerator.java b/core/src/main/java/org/jruby/java/codegen/RealClassGenerator.java
index 17e2045393..0a5e4934e9 100644
--- a/core/src/main/java/org/jruby/java/codegen/RealClassGenerator.java
+++ b/core/src/main/java/org/jruby/java/codegen/RealClassGenerator.java
@@ -821,12 +821,13 @@ public abstract class RealClassGenerator {
                     }
                 }
             } else {
+                // if the return type is not an IRubyObject implementer, coerce to that type before casting
                 if (!IRubyObject.class.isAssignableFrom(returnType)) {
                     mv.ldc(Type.getType(returnType));
                     mv.invokeinterface(
                         p(IRubyObject.class), "toJava", sig(Object.class, Class.class));
-                    mv.checkcast(p(returnType));
                 }
+                mv.checkcast(p(returnType));
                 mv.areturn();
             }
         } else {
@headius

This comment has been minimized.

Copy link
Member

commented Jun 11, 2018

@robertpanzer Can you test a master build of JRuby with the above patch and confirm it resolves your issue?

@robertpanzer

This comment has been minimized.

Copy link
Author

commented Jun 11, 2018

Sure! I’ll try that tomorrow.
Thanks a lot!

@headius

This comment has been minimized.

Copy link
Member

commented Jun 11, 2018

Actually I made it a bit easier and pushed branch interface_returns_ruby_obj.

@headius headius added the tests label Jun 11, 2018

@headius

This comment has been minimized.

Copy link
Member

commented Jun 11, 2018

This regressed in 3613060, when the checkcast was accidentally moved inside the logic for non-IRubyObject coercing.

@headius headius added this to the JRuby 9.2.1.0 milestone Jun 11, 2018

@headius headius closed this in ff90e42 Jun 12, 2018

@headius

This comment has been minimized.

Copy link
Member

commented Jun 12, 2018

I went ahead and merged it. Can you come up with a spec for spec/java_integration?

@robertpanzer

This comment has been minimized.

Copy link
Author

commented Jun 12, 2018

I just tested with the latest JRuby master and I can confirm that this issue is indeed fixed.
Thanks a lot!!!

@hebrayah

This comment has been minimized.

Copy link

commented Aug 7, 2018

Great, but when its going to be released to public ?

cescoffier added a commit to reactiverse/vertx-maven-plugin that referenced this issue Aug 8, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.