JDK19 java/lang/Thread/virtual/stress/PinALot.java#id0 IllegalMonitorStateException - RuntimeException: count = 653974 #16258
Comments
JasonFengJ9
added
test failure
project:loom
|
https://openj9-jenkins.osuosl.org/job/Test_openjdk19_j9_sanity.openjdk_ppc64le_linux_Nightly/41/ |
|
@tajila @fengxue-IS fyi. I assume a recent regression, or a test recently unexcluded. |
|
20x grinder on ppc: https://openj9-jenkins.osuosl.org/job/Grinder/1502/ |
|
https://openj9-jenkins.osuosl.org/job/Test_openjdk19_j9_sanity.openjdk_aarch64_linux_Nightly/44 |
|
20x grinder on aarch64: https://openj9-jenkins.osuosl.org/job/Grinder/1503/ |
|
I ran a grinder internally with a minor modification to ReentrantLock and I cant reproduce the failure. |
|
The issue occurs when the carrier thread is set as the owner of the lock instead of the virtual thread. Investigating why this occurs |
Interesting... crash in Scavenger #16249 I am still investigating bad (stale?) object in Nursery might be taken from |
|
https://openj9-jenkins.osuosl.org/job/Test_openjdk19_j9_sanity.openjdk_ppc64_aix_Nightly/44 |
|
https://openj9-jenkins.osuosl.org/job/Test_openjdk19_j9_sanity.openjdk_ppc64le_linux_Nightly/44 |
|
Just posting output from my grinder here so I dont lose it. |
|
@dmitripivkine I've confirmed that the issue here is the carrierThread is a VirtualThread instance |
|
@pshipton have you ever seen this on x86? Im thinking there is a memory ordering issue here. |
|
@tajila every occurrence I've seen is recorded here, so no, not on x86. I only see alinux, plinux, and once on aix. |
|
Another kind of failure |
|
so far no failures when |
|
@0xdaryl looks like disabling natinve inlining fixes this issue. Is there a way to determine if the result of Thread.currentThread() is being used as a constant value? |
|
Also, I find it interesting that I never reproduced this on x86 |
|
https://openj9-jenkins.osuosl.org/job/Test_openjdk19_j9_sanity.openjdk_ppc64_aix_Nightly/55 https://openj9-jenkins.osuosl.org/job/Test_openjdk19_j9_sanity.openjdk_x86-64_linux_Nightly/55 |
|
@tajila see the reproduction on x86 added to the previous comment. |
|
https://openj9-jenkins.osuosl.org/job/Test_openjdk19_j9_sanity.openjdk_ppc64_aix_Nightly/56 https://openj9-jenkins.osuosl.org/job/Test_openjdk19_j9_sanity.openjdk_aarch64_linux_Nightly/59 https://openj9-jenkins.osuosl.org/job/Test_openjdk19_j9_sanity.openjdk_x86-64_linux_Nightly/58 https://openj9-jenkins.osuosl.org/job/Test_openjdk19_j9_sanity.openjdk_aarch64_mac_Nightly/63 https://openj9-jenkins.osuosl.org/job/Test_openjdk19_j9_sanity.openjdk_ppc64le_linux_Nightly/60 |
|
@tajila : just wondering how the investigation here became focused on The JIT inlined implementation is the same as it always was: insert a load of the current thread object from the |
|
There are places in j.l.Thread/VirtualThread where the JCL switches from the virtualThread to the carrierThread. This is to avoid yielding twice when performing blocking operations. We are seeing that in some cases, we expect the carriedthread but currentThread returns a virtual thread. In the failure below here is the code |
|
Thanks so much for the clarification @tajila. I'll try excluding |
|
I'm able to reproduce the issue by excluding I haven't been able to repro the issue after adding more logging (Grinder |
|
Hi @a7ehuo did you also specify dontInline={java/lang/Thread.currentThread*} in those jgrinder runs? I could be wrong as it's been a while, but I'm not sure if exclude={} will prevent inlining of those methods. It may not matter (hard to imagine the aliasing isn't set up properly to ensure setCurrentThread blocks reuse of the current thread) but I'd rather say something stupid than not point out something that might matter. |
Thanks for pointing it out! I didn't add |
|
along those same lines, you may want to use a filter that excludes |
|
@a7ehuo Any luck reproducing the problem with the updated options? |
|
Although we can't reproduce the issue with a log file on Two grinders are currently running. At the moment no failure is observed.
Meanwhile, I'm also running tests to verify a proper fix. [1] |
|
Grinder run results:
|
|
I don't find any issues covering those failures. They appear to be new failures we haven't seen before, which makes it likely they are introduced by the change. |
|
@JasonFengJ9 @fengxue-IS Have you seen the failures Annabelle posted above? |
This is new.
No existing issue, the console log might have more info. |
|
Have not seen these issues previously |
For @a7ehuo's "change 2" above, this is not functionally equivalent to what she actually implemented. "Change 2" simply removed the "immutability" setting on the field which isn't completely correct. However, her PR correctly removes it and marks it "volatile" instead. It is probably worth running a 30x grinder with the actual PR to verify it is clean. |
Sounds good! I'll start the grinder runs |
|
An update on the grinder run results:
|
|
Another grinder on |
|
Thanks @a7ehuo. If I have this right, the summary is:
@pshipton : I think we're ready to merge this. |
|
Re-opening until fixed in 0.37. |
`Thread.currentThread` can be changed by JCL in JDK19 and up. Set it as volatile for JDK19 and up. Add `isJ9VMThreadCurrentThreadImmutable` in FrontEnd to query whether or not `Thread.currentThread` is immutable based on `JAVA_SPEC_VERSION`. Fixes eclipse-openj9#16258 Co-authored-by: Daryl Maier <maier@ca.ibm.com> Signed-off-by: Annabelle Huo <Annabelle.Huo@ibm.com>
|
Closing since #17021 is merged for 0.37 |
Failure link
From an internal build(
rhel8le-rt1-4):Rerun in Grinder - Change TARGET to run only the failed test targets.
Optional info
Failure output (captured from console output)
50x internal grinder - 18/100 failed
The text was updated successfully, but these errors were encountered: