JRuby adds Java proxy classes to the Java module even if they are not from JRubyClassLoader

[barrkel]

JRuby version 9.3.13.0
Platform 6.5.13-1rodete2

Expected behavior:

• There is a class org.example.C which is in a jar which is not on the class path, call this C_jar
• There is a class org.example.C which is in a jar which is on the class path, call this C_classpath
• C_jar is loaded via URLClassLoader and instantiated in Java code. This is done to isolate the different versions of C.
• Use Ruby to evaluate an object expression which results in an instance of an object with the class C_jar
• Use the Java module to reference the class C, expecting to get C_classpath (i.e. Java::OrgExample::C)

Actual behavior:

• Using the Java module syntax to access C results in C_jar

Workaround:

• Use the Java module syntax to access C before any Ruby expression which evaluates to an object of class C_jar

In Java.java, generateClassProxy calls addToJavaPackageModule without checking the class loader of the class it generated a proxy for. Effectively, manipulating values from non-JRubyClassLoader classes pollutes the Java module with incompatible classes.

[headius]

Interesting! Do you think you can come up with a small reproduction for us?

[barrkel]

java-module-pollution.zip

Steps:

1. Unzip the attached zip and enter the directory.
2. Run ./build.sh to compile the Java classes and build the jar; javac and jar need to be on the path.
3. Run ./repro.rb broken with jruby on the path and observe:

Expecting 'isolated in jar': isolated in jar
Expecting 'packaged with app': isolated in jar

4. Run ./repro.rb workaround and observe:

Expecting 'isolated in jar': isolated in jar
Expecting 'packaged with app': packaged with app

[barrkel]

Having thought about it a little bit more:

• There's a potential backward compatibility issue; it's possible some people have relied on Java:: access to classes they loaded from a jar similar to how the repro works. This may have worked OK if they didn't have duplicate classes on the app class path.
• There may be subtleties to how class loader parent/child delegation works. The scenario I present is specifically problematic when the parent of the jar's class loader is the bootstrap class loader (bootstrap is parent of the system class loader). This is required to actually get the jar's version of the class and not accidentally load the app's class. I don't know how other configurations of class loader hierarchy may interfere, and I'm not sure of the right predicate with which to gate addition of proxy classes to the Java module - if it needs a simple class loader comparison, or if it needs to follow chains, or if it needs to be configurable.

[headius]

After looking at your example I think I understand the problem.

When a Java object enters Ruby with a class we haven't seen before, we need to set up the "proxy" class under our own Java module hierarchy. If multiple classes of the same name but different effective classes (e.g. from different classloaders), whichever gets set up first will "win" and become the entry in Java::. That's why your workaround fixes it: the classloader version gets set up first.

There's no hard reason why we immediately add the constant entry for the class under Java:: other than that step being a standard part of setting up the proxy. If we didn't do that, the "side loaded" jar version of the class would not be assigned a constant entry and you'd get the expected behavior.

Avoiding setting the constant also would not really affect users, since any subsequent access of the constant name would lazily pick up the class, and only from the classloader hierarchy JRuby uses for Java integration.

An alternative, less aggressive change would be to only automatically add the constant entry when we know it's from our Java integration classloader.

@kares @enebo What are your thoughts here? It's probably not something we can change in 9.4.x but for Java 10 we could consider not always setting up the Java:: constant entries until they are explicitly requested.

[headius]

I pushed #8208 as an experiment in delaying when we set up the Java:: constants, and my first attempt appears to be passing all internal tests. Instead of immediately adding those Java:: entries when we encounter a new class, we only add them when they are explicitly accessed (as Java::C in your code) and load them only from our internal Java integration classloader hierarchy. Objects with different classes of the same name would never be accessible through that syntax, but would continue to work fine in JRuby.

@barrkel If you are able to test that branch with your application, I'd appreciate it.

[headius]

Oh and I should temper expectations a bit... this may be too major of a change to do in a point release of JRuby 9.4.x, so it would either be guarded behind a configuration property (defaulting to non-lazy in 9.4.7.0) or only merged into JRuby 10 for release later this year.

[headius]

@barrkel I have a PR settling now that would allow to turn off the "eager" constant definition, so that only constants accessed directly (and loaded out of our classloader hierarchy) will be defined in the Java:: namespace. The property will be jruby.ji.eager.constants=false for JVM or -Xjit.eager.constants=false passed to JRuby's launcher, and it defaults to true in JRuby 9.4.7.0. We'll switch it to false for JRuby 10.

[headius]

Ok two attempts later and we just made things worse! 😂

We are reverting #8368 and #8208 and will make another attempt in 9.4.10.0 or later.

The workaround remains the same: access the correct class first so it gets into the package structure.