webcompere/system-stubs

Warnings on illegal reflective access, and tests run with Java 16 fail with java.lang.reflect.InaccessibleObjectException

Closed this issue ยท 17 comments

While working on SonarSource/sonarlint-core#366, an issue triggered by Java 16's stricter rules on JDK internal access, we stumbled upon a similar issue in system-stubs - see stack trace below.

Full stack trace

java.lang.reflect.InaccessibleObjectException: Unable to make field private final java.util.Map java.util.Collections$UnmodifiableMap.m accessible: module java.base does not "opens java.util" to unnamed module @2e5c649

	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:357)
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
	at java.base/java.lang.reflect.Field.checkCanSetAccessible(Field.java:177)
	at java.base/java.lang.reflect.Field.setAccessible(Field.java:171)
	at uk.org.webcompere.systemstubs.environment.EnvironmentVariables.getFieldValue(EnvironmentVariables.java:243)
	at uk.org.webcompere.systemstubs.environment.EnvironmentVariables.getEditableMapOfVariables(EnvironmentVariables.java:205)
	at uk.org.webcompere.systemstubs.environment.EnvironmentVariables.restoreOriginalVariables(EnvironmentVariables.java:187)
	at uk.org.webcompere.systemstubs.environment.EnvironmentVariables.doTeardown(EnvironmentVariables.java:158)
	at uk.org.webcompere.systemstubs.resource.SingularTestResource.teardown(SingularTestResource.java:26)
	at uk.org.webcompere.systemstubs.resource.Resources.executeCleanup(Resources.java:59)
	at uk.org.webcompere.systemstubs.resource.Resources.execute(Resources.java:45)
	at uk.org.webcompere.systemstubs.resource.TestResource.execute(TestResource.java:31)
	at uk.org.webcompere.systemstubs.resource.Executable.execute(Executable.java:30)
	at org.sonarsource.sonarlint.core.telemetry.TelemetryHttpClientTests.failed_optout_payload_should_log_if_debug(TelemetryHttpClientTests.java:144)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:567)
	at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:688)
	at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
	at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:84)
	at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$6(TestMethodTestDescriptor.java:210)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:206)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:131)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:65)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:129)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:127)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:126)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:84)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:143)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:129)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:127)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:126)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:84)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:143)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:129)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:127)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:126)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:84)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:32)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:51)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:108)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:88)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:67)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:52)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:96)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:75)
	at com.intellij.junit5.JUnit5IdeaTestRunner.startRunnerWithArgs(JUnit5IdeaTestRunner.java:71)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:221)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:54)

A possible workaround is to pass the --illegal-access=warn argument to the JVM, though this is not very future-proof.

@jblievremont in JVMs below 16, we get a warning for this. At this stage, I don't know of an obvious workaround, given that this library's method is to hack into what should be an immutable collection.

If anyone has any good ideas for how else environment variables could be manipulated in later JVMs, then please post them here and I'll try them out.

Happy to accept PRs on the subject too.

This is the same root cause as produces:

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by uk.org.webcompere.systemstubs.environment.EnvironmentVariables (file:/Users/user/.m2/repository/uk/org/webcompere/system-stubs-core/1.2.0/system-stubs-core-1.2.0.jar) to field java.util.Collections$UnmodifiableMap.m
WARNING: Please consider reporting this to the maintainers of uk.org.webcompere.systemstubs.environment.EnvironmentVariables
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

At the moment the only methodology used across various libraries that coerce environment variables into being writable is:

  • Get hold of the immutable map that holds the environment variables
  • Use reflection to make the inner map field no longer private
  • Hack into that inner map field and write some values into it

The outcome is that we can write environment variables at runtime, even though Java has hidden them behind a proxy object that's trying to keep them read-only.

The Java module system wants to stop us doing this, but it only warns us about this in earlier versions.

Our hacking into these runtime objects will need to become more powerful in the near future.

@ashleyfrieze in Java17 it is no longer a warning...

@McKratt - yeah - there's presently not a solution for this issue. Keep an eye on this issue. When there's a fix, we'll update it.

To the best of my knowledge, all the major libraries that allow mutation of environment variables for test use the same technique.

This issue is now resolved. Note the new version 2.0.0 and the dependency on mockito-inline

Awesome, thank you! And thanks also for the --add-opens workaround. Looking forward to testing v2.0.0!

@jblievremont just waiting for the propagation to maven central. It may take a few more hours to show up on searches, but it should be downloadable with the correct version in the dependency. Read the README.md to find out what else you need to add to your Pom/build.gradle

Unfortunately, this seems to trigger mockito/mockito#2522 for those using Groovy in a particular way. mockito-inline and Groovy don't seem to co-exist nicely at all right now? :-( Just a PSA, not a complaint :-)

@chadlwilson - do we know if that affects ALL versions of mockito? System Stubs depends on 3.12.x, but could also accept 4.x

Good question @ashleyfrieze . I think so but not 100% sure. Initially the bump of system stubs brought in mixed versions (3.12.4 of inline, 4.2.0 of core which we use and control versions of already) and my first thought was some clash. After ensuring both were 4.2.0, it definitely seems at least 4.2.0 is affected.

I haven't tried a simple reprod yet, because as it turns out we need to add opens for our tests on JDK 17 anyway due to some of our own code (and old Javassist) that rely on some dodgy reflection anyway, so system stubs was not blocking us in this regard.

@chadlwilson breaking the dependency on mockito would be useful in the long term. If it becomes close to being a deal breaker, please raise a separate issue (if there isn't one open by then!).

FWIW, the upstream issue has been resolved now in mockito 4.5.0 and things seem to be working fine for us now with mockito-inline in-the-mix along with Groovy tests.

@chadlwilson - that's great news. I'll drop a note in the README.md

You may want to not bother unless someone else reports similar challenges :-)

It's possible that in my case I misdiagnosed the problem due to the similarity of the issue and symptoms. Even after that upstream mockito fix I discovered that there were some method overloads that became ambiguous from mockito and Groovy's perspective once mockito-inline was there which I needed to disambiguate since mockito-inline can mock private methods.

I didn't actually try going back to the various older mockito/mockito-inline versions to see whether it was only the disambiguation which was needed :-/

@chadlwilson - there's no harm in sharing your advice on mockito 4.5.0 as a known good version, though. It's in the README. Thanks for the info.