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

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

Closed
jblievremont opened this issue Apr 20, 2021 · 17 comments
Assignees
Labels
help wanted Extra attention is needed jdk fundamental An issue which requires deep knowledge of jdk to fix

Comments

@jblievremont
Copy link

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.

@ashleyfrieze
Copy link
Member

@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.

@ashleyfrieze ashleyfrieze changed the title Tests run with Java 16 fail with java.lang.reflect.InaccessibleObjectException Warnings on illegal reflective access, and tests run with Java 16 fail with java.lang.reflect.InaccessibleObjectException Aug 20, 2021
@ashleyfrieze
Copy link
Member

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

@ashleyfrieze
Copy link
Member

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.

@McKratt
Copy link

McKratt commented Oct 26, 2021

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

@ashleyfrieze
Copy link
Member

@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.

@ashleyfrieze ashleyfrieze added help wanted Extra attention is needed jdk fundamental An issue which requires deep knowledge of jdk to fix labels Nov 2, 2021
@ashleyfrieze
Copy link
Member

Worth noting this - stefanbirkner/system-lambda#23 (comment)

@ashleyfrieze
Copy link
Member

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

@jblievremont
Copy link
Author

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

@ashleyfrieze
Copy link
Member

@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

@ashleyfrieze ashleyfrieze self-assigned this Jan 9, 2022
@chadlwilson
Copy link
Contributor

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 :-)

@ashleyfrieze
Copy link
Member

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

@chadlwilson
Copy link
Contributor

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.

@ashleyfrieze
Copy link
Member

@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!).

@chadlwilson
Copy link
Contributor

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.

@ashleyfrieze
Copy link
Member

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

@chadlwilson
Copy link
Contributor

chadlwilson commented Apr 26, 2022

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 :-/

@ashleyfrieze
Copy link
Member

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed jdk fundamental An issue which requires deep knowledge of jdk to fix
Projects
None yet
Development

No branches or pull requests

4 participants