Agent opens access to java.base to unnamed module

[DanielThomas]

We're migrating to JDK 17 and found that we were unable to run tests with JaCoCo to verify code was not accessing encapsulated Java internals in the java.base package.

Steps to reproduce

• JaCoCo version: 0.8.8
• Operating system: macOS 10.4
• Tool integration: Command-line Agent

Given class IllegalAccess.java and JDK 17:

import java.lang.Throwable;

public class IllegalAccess {
	public static void main(String[] args) throws Exception {
		Throwable.class.getDeclaredMethod("setCause", Throwable.class).setAccessible(true);
	}
}

Run without Jacoco and note the exception:

$ java IllegalAccess
Exception in thread "main" java.lang.reflect.InaccessibleObjectException: Unable to make final void java.lang.Throwable.setCause(java.lang.Throwable) accessible: module java.base does not "opens java.lang" to unnamed module @a803f94
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354)
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
	at java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:199)
	at java.base/java.lang.reflect.Method.setAccessible(Method.java:193)
	at IllegalAccess.main(IllegalAccess.java:5)

Run with Jacoco and no exception is thrown:

$ java -javaagent:/Users/dannyt/Downloads/jacoco-0.8.8/lib/jacocoagent.jar IllegalAccess

Expected behaviour

The agent should not cause runtime side effects to code under test. This in particular makes it impossible to verify production code doesn't perform illegal accesses at test time causing this code to fail when deployed.

Actual behaviour

The agent adds java.base to the exported modules for the unnamed module causing the entire classpath to be granted access.

[marchof]

Hi @DanielThomas unfortunatelly it is not possible to implement a Java agent with on-the-fly instrumentation without any side effects. We need a way of communication between the instrumented classes and the JaCoCo runtime to collect coverage data. Please see "Coverage Runtime Dependency" in the implementation design chapter.

Any ideas how to implement the agent without adding dependencies are welcome!

You can use offline instrumentation. In this case you have to set-up the runtime dependency manually.

[DanielThomas]

@marchof I couldn't find it documented, but I believe you can use Add-Opens in the manifest and you wouldn't need that code at all. It appears that's how NewRelic got their agent working for JDK 16: newrelic/newrelic-java-agent#366

[marchof]

@DanielThomas As said before, contributions for this are welcome! If you build JaCoCo there is a quite extensive (integration) test suite, so you will immediately see whether other approaches work.

[Godin]

I believe you can use Add-Opens in the manifest

Quoting https://openjdk.org/jeps/261

Add-Opens attribute has the same meaning as the command-line option --add-opens <module>/<package>=ALL-UNNAMED.

So definitely not something that will help to solve the issue described here 😉

Moreover quoting the same https://openjdk.org/jeps/261

These attributes are interpreted only in the main executable JAR file of an application, i.e., in the JAR file specified to the -jar option of the Java run-time launcher; they are ignored in all other JAR files.

---

Unfortunately while implementing #829 we forgot that the agent is loaded by the application class loader and thus in the same unnamed module causing issue described here.

If the agent will have a different module, the problem described here will go away.

We asked whether agent can be a real module (not unnamed) in https://groups.google.com/g/jacoco-dev/c/NNP863PDuto/m/OVrNp2WUAAAJ
And as an answer were directed to http://mail.openjdk.java.net/pipermail/jigsaw-dev/2017-June/012883.html

There isn’t any support at this time to develop or deploy java agents as modules. It has been prototyped and most of the issues are interactions are understood, it’s just too much to take on for Java SE 9.

Unfortunately corresponding ticket https://bugs.openjdk.org/browse/JDK-6932391 is still open nowadays 😞

Fortunately https://docs.oracle.com/javase/specs/jvms/se11/html/jvms-5.html#jvms-5.3.6 states

A class loader’s unnamed module is distinct from all run-time modules (including unnamed modules) bound to other class loaders.

🎉

To demonstrate this:

import java.io.IOException;
import java.io.InputStream;

public final class IllegalAccess {
  public static void main(String[] args) throws Exception {
    new ClassLoader() {
      @Override
      protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
        if (name.equals(IllegalAccess.class.getName())) {
          byte[] b;
          try (InputStream r = getResourceAsStream(name + ".class")) {
            b = r.readAllBytes();
          } catch (IOException e) {
            throw new RuntimeException(e);
          }
          defineClass(name, b, 0, b.length);
        }
        return super.loadClass(name, resolve);
      }
    }.loadClass(IllegalAccess.class.getName())
      .getMethod("run")
      .invoke(null);
  }

  public static void run() throws Exception {
    Throwable.class.getDeclaredMethod("setCause", Throwable.class).setAccessible(true);
  }
}

produces

Exception in thread "main" java.lang.reflect.InvocationTargetException
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:568)
        at IllegalAccess.main(IllegalAccess.java:22)
Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make final void java.lang.Throwable.setCause(java.lang.Throwable) accessible: module java.base does not "opens java.lang" to unnamed module @71bc1ae4
        at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354)
        at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
        at java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:199)
        at java.base/java.lang.reflect.Method.setAccessible(Method.java:193)
        at IllegalAccess.run(IllegalAccess.java:26)
        ... 5 more

even when executed with JaCoCo agent.

So this can be used for our InjectedClassRuntime - I drafted PoC in #1334

[Godin]

This in particular makes it impossible to verify production code doesn't perform illegal accesses at test time

Not impossible - you can run tests without JaCoCo agent for such verification 😉

However I fully agree that will be better to have the ability to do such verification without disabling JaCoCo.

And find it interesting that this was unnoticed for more than three years - does it mean presence of
--add-opens java.base/java.lang=ALL-UNNAMED in deployments? 🙈 😆

@DanielThomas Thank you for reporting this! 👍 🤗 ❤️

[DanielThomas]

@Godin amazing, that's a great find! Temporarily turning off the agent is indeed not a big deal because once we've dealt with illegal accesses in our migration, I doubt they'll regress again. I only found this because I fixed this in Gradle and wondered why I still couldn't get my test to fail:

gradle/gradle#19771

Yeah, I'd guess folks are adding opens everywhere. We're trying to avoid that and so far, it's all backwards compatible library upgrades or small fixes to internal code.

Really appreciated. Do you or the project have sponsorships somewhere or have a cause you'd like me to donate to on your behalf to say thanks?