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

newInstance on protected/private classes sometimes give an Index OOB Exception #6079

Closed
byteit101 opened this issue Feb 18, 2020 · 9 comments · Fixed by #6091
Closed

newInstance on protected/private classes sometimes give an Index OOB Exception #6079

byteit101 opened this issue Feb 18, 2020 · 9 comments · Fixed by #6091
Assignees

Comments

@byteit101
Copy link
Member

@byteit101 byteit101 commented Feb 18, 2020

Provide at least:

  • Version: jruby 9.2.9.0 (2.5.7) 2019-10-30 458ad3e Java HotSpot(TM) 64-Bit Server VM 11.0.5+10-LTS on 11.0.5+10-LTS +jit [linux-x86_64]
  • RHEL7 (Linux 3.10.0-1062.9.1.el7.x86_64 #1 SMP Mon Dec 2 08:31:54 EST 2019 x86_64 x86_64 x86_64 GNU/Linux)

Other relevant info you may wish to add:

  • Installed or activated gems: jbundler + this jarfile:
AETHER_VERSION = '1.4.1'
MAVEN_VERSION = '3.6.2'
LOGGER_VERSION = '1.7.30'
jar 'org.apache.maven.resolver:maven-resolver-impl', AETHER_VERSION
jar 'org.apache.maven.resolver:maven-resolver-transport-file', AETHER_VERSION
jar 'org.apache.maven.resolver:maven-resolver-transport-http', AETHER_VERSION
jar 'org.apache.maven.resolver:maven-resolver-connector-basic', AETHER_VERSION

# maven-resolver-provider is the source of the class, but I'm not sure if the rest of the jars are needed or not
jar 'org.apache.maven:maven-resolver-provider', MAVEN_VERSION
jar 'org.apache.maven:maven-settings-builder', MAVEN_VERSION
jar 'org.apache.maven:maven-model-builder', MAVEN_VERSION
jar 'org.slf4j:slf4j-jdk14', LOGGER_VERSION

Expected Behavior
The following code prints every time (excepting object id changes):

#<Java::JavaConstructor(org.eclipse.aether.RepositorySystemSession,org.eclipse.aether.RequestTrace,java.lang.String,org.eclipse.aether.impl.ArtifactResolver,org.eclipse.aether.impl.VersionRangeResolver,org.eclipse.aether.impl.RemoteRepositoryManager,java.util.List)>
#<Java::JavaObject:0x654b899f>
#<Java::OrgApacheMavenRepositoryInternal::DefaultModelResolver:0x13f4048e>

Script:

require "bundler/setup"
require 'jbundler'

x = java.util.ArrayList.new([])
# DefaultModelResolver is package private, make public and initialize
ctor = org.apache.maven.repository.internal.DefaultModelResolver.java_class.declared_constructors[0]
ctor.accessible=true
p ctor
res = ctor.new_instance(nil, nil, nil, nil, nil, nil, x)

p res
p res.to_java

Actual Behavior
Sometimes it works, sometimes it throws an index out of bounds exception:

$ ruby test.rb
#<Java::JavaConstructor(org.eclipse.aether.RepositorySystemSession,org.eclipse.aether.RequestTrace,java.lang.String,org.eclipse.aether.impl.ArtifactResolver,org.eclipse.aether.impl.VersionRangeResolver,org.eclipse.aether.impl.RemoteRepositoryManager,java.util.List)>
#<Java::JavaObject:0x4eac65db>
#<Java::OrgApacheMavenRepositoryInternal::DefaultModelResolver:0x7884f722>
$ ruby test.rb
#<Java::JavaConstructor(org.apache.maven.repository.internal.DefaultModelResolver)>
Unhandled Java exception: java.lang.ArrayIndexOutOfBoundsException: Index 6 out of bounds for length 1
java.lang.ArrayIndexOutOfBoundsException: Index 6 out of bounds for length 1
            convertArguments at org/jruby/javasupport/JavaUtil.java:553
            convertArguments at org/jruby/javasupport/JavaCallable.java:143
                new_instance at org/jruby/javasupport/JavaConstructor.java:216
                        call at org/jruby/javasupport/JavaConstructor$INVOKER$i$0$0$new_instance.gen:-1
                cacheAndCall at org/jruby/runtime/callsite/CachingCallSite.java:343
                        call at org/jruby/runtime/callsite/CachingCallSite.java:76
  invokeOther17:new_instance at test.rb:11
                      <main> at test.rb:11
         invokeWithArguments at java/lang/invoke/MethodHandle.java:710
                        load at org/jruby/ir/Compiler.java:89
                   runScript at org/jruby/Ruby.java:857
                 runNormally at org/jruby/Ruby.java:780
                 runNormally at org/jruby/Ruby.java:798
                 runFromMain at org/jruby/Ruby.java:610
               doRunFromMain at org/jruby/Main.java:412
                 internalRun at org/jruby/Main.java:304
                         run at org/jruby/Main.java:234
                        main at org/jruby/Main.java:206

$ ruby test.rb
#<Java::JavaConstructor(org.eclipse.aether.RepositorySystemSession,org.eclipse.aether.RequestTrace,java.lang.String,org.eclipse.aether.impl.ArtifactResolver,org.eclipse.aether.impl.VersionRangeResolver,org.eclipse.aether.impl.RemoteRepositoryManager,java.util.List)>
#<Java::JavaObject:0x1bc6c20d>
#<Java::OrgApacheMavenRepositoryInternal::DefaultModelResolver:0x39b85a73>
$ ruby test.rb
#<Java::JavaConstructor(org.eclipse.aether.RepositorySystemSession,org.eclipse.aether.RequestTrace,java.lang.String,org.eclipse.aether.impl.ArtifactResolver,org.eclipse.aether.impl.VersionRangeResolver,org.eclipse.aether.impl.RemoteRepositoryManager,java.util.List)>
#<Java::JavaObject:0x7a730479>
#<Java::OrgApacheMavenRepositoryInternal::DefaultModelResolver:0x654b899f>
$ ruby test.rb
#<Java::JavaConstructor(org.eclipse.aether.RepositorySystemSession,org.eclipse.aether.RequestTrace,java.lang.String,org.eclipse.aether.impl.ArtifactResolver,org.eclipse.aether.impl.VersionRangeResolver,org.eclipse.aether.impl.RemoteRepositoryManager,java.util.List)>
#<Java::JavaObject:0x654b899f>
#<Java::OrgApacheMavenRepositoryInternal::DefaultModelResolver:0x13f4048e>
$ ruby test.rb
#<Java::JavaConstructor(org.apache.maven.repository.internal.DefaultModelResolver)>
Unhandled Java exception: java.lang.ArrayIndexOutOfBoundsException: Index 6 out of bounds for length 1
java.lang.ArrayIndexOutOfBoundsException: Index 6 out of bounds for length 1
            convertArguments at org/jruby/javasupport/JavaUtil.java:553
            convertArguments at org/jruby/javasupport/JavaCallable.java:143
                new_instance at org/jruby/javasupport/JavaConstructor.java:216
                        call at org/jruby/javasupport/JavaConstructor$INVOKER$i$0$0$new_instance.gen:-1
                cacheAndCall at org/jruby/runtime/callsite/CachingCallSite.java:343
                        call at org/jruby/runtime/callsite/CachingCallSite.java:76
  invokeOther17:new_instance at test.rb:11
                      <main> at test.rb:11
         invokeWithArguments at java/lang/invoke/MethodHandle.java:710
                        load at org/jruby/ir/Compiler.java:89
                   runScript at org/jruby/Ruby.java:857
                 runNormally at org/jruby/Ruby.java:780
                 runNormally at org/jruby/Ruby.java:798
                 runFromMain at org/jruby/Ruby.java:610
               doRunFromMain at org/jruby/Main.java:412
                 internalRun at org/jruby/Main.java:304
                         run at org/jruby/Main.java:234
                        main at org/jruby/Main.java:206


This is different to #449 as that deals with ruby classes generated at runtime

@headius
Copy link
Member

@headius headius commented Feb 19, 2020

Very peculiar. I'm not sure why it would only fail some of the time.

@byteit101
Copy link
Member Author

@byteit101 byteit101 commented Feb 19, 2020

Augh, I missed the fact that there is more than one ctor, and one has 1 argument!
Should be .declared_constructors.find{|dc|dc.argument_types.length == 7}
Bug on my part, though I'll let you decide if you want to keep this open to improve the error message to a typical ArgumentError

@headius
Copy link
Member

@headius headius commented Feb 19, 2020

Yeah there's still a bug here...some arity check must be missing.

@headius
Copy link
Member

@headius headius commented Feb 21, 2020

Without an arity check somewhere before it, this code is clearly going to run into trouble:

public static Object[] convertArguments(final IRubyObject[] args, final Class<?>[] types, int offset) {
final Object[] arguments = new Object[ args.length - offset ];
for ( int i = arguments.length; --i >= 0; ) {
arguments[i] = args[ i + offset ].toJava( types[i] );
}
return arguments;
}

I'm checking to see if there's any arity check along your error call path, and if not add one.

headius added a commit to headius/jruby that referenced this issue Feb 21, 2020
If this call were made with the wrong number of arguments, it
could easily walk off the type array due to the lack of arity
checking in the convertArguments call.

Fixes jruby#6079.
@headius
Copy link
Member

@headius headius commented Feb 21, 2020

Ok, so wrapping this up...

The original error happened because sometimes, either randomly on the same JVM or by using a different JVM or sequence of calls, the zero'th element of the declared_constructors array may not be the one you expected. When it's the seven-argument version, your code works. When it's a different version, it might error.

The bug was that when calling a JavaConstructor via new_instance, we did not arity-check before attempting to convert all arguments to the appropriate Java types. If more arguments were passed than the constructor expected, it would walk off the end of the type array.

The fix in #6091 will prevent that error. On your end, you should make sure you have the right Java constructor in hand before invoking it.

As a side note, the API you used is supposed to be deprecated; the whole JavaObject pyramid is a relic from days when most of the Java integration magic was implemented in Ruby with a few cumbersome "mirror" classes to represent the Java types (as opposed to now, where we bind all Java methods like Ruby methods and dispatch them internally). Normally I'd say you should use java_method or java_send to access private or protected methods, but I don't think there's an equivalent way to get a constructor.

@enebo Two questions...

  1. Should we add such a mechanism, like java_constructor?
  2. Should this case have a test, give that these APIs are deprecated?

@headius headius added this to the JRuby 9.2.11.0 milestone Feb 21, 2020
@headius
Copy link
Member

@headius headius commented Feb 21, 2020

I should say... the fix in #6091 will prevent the opaque ArrayIndexOutOfBoundsException by more properly raising a Ruby ArgumentError.

@enebo
Copy link
Member

@enebo enebo commented Feb 21, 2020

I was wondering if classloading/timing of Java being mirrored is a possible issue here?

  1. I think so? unless we have a special :new for method...but Java can have a .new() so seems like a conflict.
  2. tests are never bad but I guess anything which actually still works in theory should have a test. Until we flat out remove it perhaps we should know it still works.

@headius
Copy link
Member

@headius headius commented Feb 21, 2020

  1. Using :new as a special method name seems great to me. Constructors don't have a consistent name otherwise. So it would be ArrayList.java_method(:new).invoke. I can live with that.

@headius
Copy link
Member

@headius headius commented Feb 21, 2020

Oops

  1. Yeah I was leaning this way.

@headius headius self-assigned this Feb 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants