-
Notifications
You must be signed in to change notification settings - Fork 721
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
Mockito fails with "cannot mock" exceptions in OpenJ9 11 and 17 #17454
Comments
@gacholio can you please take a look at this |
The only classes that should be unmodifiable are ones annotated with |
Can you please try a more modern VM? The stated one is over a year old. The change to use the annotation claims to be 4 years old, so I'm not sure this will actually fix anything, but it's worth a try. |
@gacholio I tried running the mockito test suite just now using the latest IBM Semeru Java 17 runtime available at . This one:
The mockito tests still fail in the same way. For example, here's one failure message.
Unfortunately the UnmodifiableClassException doesn't say which class was unmodifiable, and it is coming from a method (retransformClasses) that takes a list of classes to transform. But I strongly suspect it is java.lang.Object. By the way, if the only unmodifiable classes really are just the three that you mention, then there's an error in the comment on the classIsModifiable method in openj9/runtime/util/hshelp.c, which says:
|
The comment is outdated - you should be able to transform Object. I will make sure this is true. |
@yogregg I've been able to reproduce this issue with |
If you can reproduce this, the next step is to modify a VM to print which class causes the problem. I'll produce a patch and build. |
@tajila regarding your question about running a single tests, I've always been a maven user, and never used gradle until my attempts to illustrate this issue using mockito's own tests. But https://stackoverflow.com/questions/22505533/how-to-run-only-one-unit-test-class-using-gradle and other articles indicate that there's a --tests argument you can use to do that. I've also learned that gradle does a lot of caching and dependency analysis on what it runs, and that if you rerun "gradlew build" or "gradlew test" when no relevant code has changed, it won't actually rerun the tests, even though it the test reports directory will still contain results. I've found that "gradlew test --rerun-tasks" forces the tests to rerun, which I found useful in repeatedly running them to try to debug, even when I hadn't changed code. I don't know if that's how an experienced gradle user would do it, but it seems to work for me. |
Patch: master...gacholio:openj9:unmod Java 8 internal build: http://vmfarm.rtp.raleigh.ibm.com/build_info.php?build_id=52410 Java11 open build: https://hyc-runtimes-jenkins.swg-devops.com/view/OpenJ9%20-%20Personal/job/Pipeline-Build-Test-Personal/16833/ |
@gacholio has anyone tried reproducing this with the instrumented internal build yet to verify the unmodifiable class, or made other progress on diagnosing the issue? |
Apologies - Tobi has been away and I've been off sick the last couple of days. Hopefully we'll have an update by end of week. |
@gacholio I tried it again, none of the |
@yogregg Turns out we have not reproduced the reported failure locally. The runs we've done report lots of errors (and at least one VM crash which will be addressed seperately), but none of the failures are the unmodifiable class issue. |
@gacholio When I was trying to reproduce this originally I was having trouble with getting gradle to actually rerun tests consistently. “gradlew test” didn’t actually rerun even though it appeared to. "gradlew test --rerun-tasks" seemed to do the trick for me. Is it possible that’s behind it appearing to be reproducible at first but not now? Also so that I can more carefully compare your test results and mine would it be possible for you to attach the contents of mockito's top level build/reports/tests/test directory? And are you able to reproduce with the publicly available recent Semeru runtime I used? I am on vacation now and can’t look more carefully at the moment at some way to run just one specific test that I’m seeing this problem on. |
I ran on windows with
and the result was
a few others |
|
The test above was run with |
@tajila I tried running the mockito tests today with the same Windows Semeru 17.0.7+7 version that you used, and I am now seeing results more like yours: some JVM crashes, plus many NullPointerExceptions in ArrayList. On quite a few tries, the test runs never finished at all (maybe related to the JVM crashes), and even when I did get lucky and have it finish, I didn't get any test results populated into the build/reports/tests/test directory, so I could only see the results from the last few tests that I could still see in my shell window. So I don't know whether anything is still failing with the UnmodifiableClassException. I tried again on the same sources (up to date mockito head as of today) with an Oracle Java 17 and only got two test failures, both very different, and I suspect that's just a real failure in the current unreleased mockito sources. So, while the behavior seems to have changed in newer versions, mockito is still very broken on Semeru Java, and I hope that you can get to the bottom of it. I gather that the new failure symptoms that you and I are both now seeing are something that you can reproduce. |
It might be worth taking the JIT out of the equation with:
(or the equivalent on non-windows) before running. |
Or not - I still see loads of the same failures running without the JIT. |
I was able to reproduce the original failures
In every case it was |
I've tracked the code to https://github.com/mockito/mockito/blob/222c14c97bc2ec59ec5e486ddaa6438edded0e5c/src/main/java/org/mockito/internal/creation/bytebuddy/InlineBytecodeGenerator.java#L266 which adds the super class to the list of classes that are retransformed |
Looking at j/l/Object
|
The code you quote is for base type and array ROM classes - Object is created from a class file. |
The only place that sets the bit for created classes is: openj9/runtime/bcutil/ROMClassBuilder.cpp Lines 1254 to 1256 in 83da51b
which is set in two possible places: openj9/runtime/bcutil/ClassFileOracle.cpp Lines 559 to 561 in 83da51b
openj9/runtime/bcutil/ClassFileOracle.cpp Line 222 in 83da51b
The constructor one (which I had previously missed) comes from: openj9/runtime/bcutil/ROMClassCreationContext.hpp Lines 243 to 258 in 83da51b
Object is unmodifiable if extensions are being used, which I expect is not the case here. I believe the NULL check is basically checking that this is the first class being loaded, though it's a pretty hacky way of doing so. |
I'll do some startup debug and see what's happening. |
The code appears to be doing what it intends. One side-effect: running |
Without
with
|
Extensions enabled if openj9/runtime/util/extendedHCR.c Lines 25 to 76 in 83da51b
|
It would be better to always mark Object as modifiable, but reject extended changes at HCR time (the extensions enabled but not used case). I'm not immediately sure how difficult this would be in the current framework). |
@tajila You've looked into cores - why are we in extended mode (if we are)? |
We are not in extended HCR mode
|
is not set, which implies |
There is also
|
That core was running with -Xint |
I'll prototype the change to disallow extended changes to Object while leaving it generally modifiable. |
https://github.com/gacholio/openj9/tree/objectmod Passes internal testing, but it's worth noting that we do not test the |
Currently, Object is marked unmodifiable if extended HCR is enabled. This change allows Object to be modified, but disallows use of the extensions on Object. Fixes: eclipse-openj9#17454 Signed-off-by: Graham Chapman <graham_chapman@ca.ibm.com>
Currently, Object is marked unmodifiable if extended HCR is enabled. This change allows Object to be modified, but disallows use of the extensions on Object. Fixes: eclipse-openj9#17454 Signed-off-by: Graham Chapman <graham_chapman@ca.ibm.com>
Currently, Object is marked unmodifiable if extended HCR is enabled. This change allows Object to be modified, but disallows use of the extensions on Object. Fixes: eclipse-openj9#17454 Signed-off-by: Graham Chapman <graham_chapman@ca.ibm.com>
Java -version output
openjdk version "11.0.17" 2022-10-18
IBM Semeru Runtime Open Edition 11.0.17.0 (build 11.0.17+8)
Eclipse OpenJ9 VM 11.0.17.0 (build openj9-0.35.0, JRE 11 Windows 10 amd64-64-Bit Compressed References 20221031_487 (JIT enabled, AOT enabled)
OpenJ9 - e04a7f6
OMR - 85a21674f
JCL - a94c231303 based on jdk-11.0.17+8)
Summary of problem
The mockito Java mocking library frequently throws "cannot mock" exceptions when using OpenJ9 that I have not seen on other Java implementations. This problem happens with at least OpenJ9 11 and 17 and at least on both Windows and Linux. The Java version output above it just one of many failing versions I've tried. Here's an example exception:
This is easy to reproduce from publicly available sources, because the mockito library's own unit tests have hundreds of failures of this sort when the tests are run with OpenJ9. You just have to make a minor modification to mockito's top-level build.gradle, adding lines like this (referring to your own Java location):
You can see more details on this including a zip file of mockito's test result reports when it is run this way in mockito/mockito#2995
I'm reporting this here as well because this problem sounds possibly related to #3410 That issue refers to a change made in gacholio@71bed50 that apparently resolved the problem for some, but I'm still seeing similar failures. The issue there was that java.lang.Object was not a modifiable class in OpenJ9 (classIsModifiable method in https://github.com/eclipse-openj9/openj9/blob/openj9-0.36.0/runtime/util/hshelp.c), which is consistent with the error stack. That version is the modified version that apparently helped some people, but it still mentions in its comments that Object can't be modified in "extended HCR mode" (I don't know what that is).
This is getting in the way of testing our product on OpenJ9, which makes it impossible to say to our customers with any confidence that it is safe for them to use OpenJ9.
What is the source of this problem? Am I right to suspect that it is related to an ongoing but narrower inability to modify Object? If so, what is "extended HCR mode" and is there some way to control whether we are using that or not, or is there some other fix or workaround for this?
The text was updated successfully, but these errors were encountered: