Bug ID: JDK-8344199 Incorrect excluded field value set by getEventWriter intrinsic

Type: Bug
Component: hotspot
Sub-Component: compiler
Affected Version: 19,21,23,24

Priority: P3
Status: Resolved
Resolution: Fixed

Submitted: 2024-11-14
Updated: 2025-03-07
Resolved: 2024-11-19

JDK 24
24 b25Fixed

Test jdk/jfr/jvm/TestVirtualThreadExclusion.java fails with VM option -Xcomp both on linux-x64 and linux-aarch64, the test log snippet:

Event:jdk.VirtualThreadEnd {
  startTime = 00:52:34.766 (2024-11-14)
  javaThreadId = 41
}


STDERR:
java.lang.NullPointerException: Cannot invoke "jdk.jfr.consumer.RecordedThread.getJavaThreadId()" because the return value of "jdk.jfr.consumer.RecordedEvent.getThread()" is null
    at jdk.jfr.jvm.TestVirtualThreadExclusion.verifyThreadExclusion(TestVirtualThreadExclusion.java:75)
    at jdk.jfr.jvm.TestVirtualThreadExclusion.main(TestVirtualThreadExclusion.java:69)

Hi Matthias, Can you reproduce it? But sure, please file a new bug.
07-03-2025
Hi we had today in our CI on Linux aarch64 a somewhat similar looking issue , but the test was jdk/jfr/event/runtime/TestThreadCpuTimeEvent.java : java.lang.NullPointerException: Cannot invoke "jdk.jfr.consumer.RecordedThread.getJavaName()" because the return value of "jdk.jfr.consumer.RecordedEvent.getThread()" is null at jdk.jfr.event.runtime.TestThreadCpuTimeEvent.lambda$generateEvents$0(TestThreadCpuTimeEvent.java:151) at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:196) at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1716) at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:570) at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:560) at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921) at java.base/java.util.stream.ReduceOps$5.evaluateSequential(ReduceOps.java:258) at java.base/java.util.stream.ReduceOps$5.evaluateSequential(ReduceOps.java:248) at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:265) at java.base/java.util.stream.ReferencePipeline.count(ReferencePipeline.java:750) at jdk.jfr.event.runtime.TestThreadCpuTimeEvent.generateEvents(TestThreadCpuTimeEvent.java:152) at jdk.jfr.event.runtime.TestThreadCpuTimeEvent.testSimple(TestThreadCpuTimeEvent.java:174) at jdk.jfr.event.runtime.TestThreadCpuTimeEvent.main(TestThreadCpuTimeEvent.java:58) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104) at java.base/java.lang.reflect.Method.invoke(Method.java:565) at com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138) at java.base/java.lang.Thread.run(Thread.java:1447) Should I open a separate JBS issue for this ?
07-03-2025
Changeset: 9d60300f Branch: master Author: Tobias Hartmann <thartmann@openjdk.org> Date: 2024-11-19 10:01:49 +0000 URL: https://git.openjdk.org/jdk/commit/9d60300feea12d353fcd6c806b196ace2df02d05
19-11-2024
Great, thanks for taking care of the fix [~thartmann]!
18-11-2024
A pull request was submitted for review. Branch: master URL: https://git.openjdk.org/jdk/pull/22195 Date: 2024-11-18 10:09:54 +0000
18-11-2024
[~pchilanomate] I sent this for review and added you as a contributor. Thanks!
18-11-2024
ILW = Incorrect result with C2 intrinsic, getEventWriter intrinsic, -XX:DisableIntrinsic=_getEventWriter = HLM = P3
18-11-2024
[~pchilanomate] You are right, it's missing shifts in the intrinsic. Quick fix: diff --git a/src/hotspot/share/opto/library_call.cpp b/src/hotspot/share/opto/library_call.cpp index 226fddce29b..fad384b9509 100644 --- a/src/hotspot/share/opto/library_call.cpp +++ b/src/hotspot/share/opto/library_call.cpp @@ -3425,6 +3425,10 @@ bool LibraryCallKit::inline_native_getEventWriter() { vthread_compare_io->init_req(_false_path, input_io_state); tid->init_req(_true_path, _gvn.transform(vthread_tid)); tid->init_req(_false_path, _gvn.transform(thread_obj_tid)); + + vthread_is_excluded = _gvn.transform(new URShiftINode(vthread_is_excluded, intcon(15))); + threadObj_is_excluded = _gvn.transform(new URShiftINode(threadObj_is_excluded, intcon(15))); + exclusion->init_req(_true_path, _gvn.transform(vthread_is_excluded)); exclusion->init_req(_false_path, _gvn.transform(threadObj_is_excluded)); pinVirtualThread->init_req(_true_path, _gvn.transform(continuation_support)); It's not a recent regression but has been wrong since the intrinsic was added in JDK 19 with JDK-8284161.
18-11-2024
Does not reproduce anymore with '-XX:DisableIntrinsic=_getEventWriter' so it does indeed seem to be a problem with the C2 intrinsic.
18-11-2024
Since the test only fails with c2 and given the exclusion mechanism doesn’t seem to be working, the getEventWriter intrinsic looks like a good candidate for the source of the bug. It returns an instance of EventWriter if already present, and it sets the exclude field if the associated thread is excluded. Looking at the generated code I see we set the excluded field with: ;; B12: # out( B13 ) <- in( B11 ) Freq: 0.05 0x0000705314db372a: mov %bpl,0xe(%rax) but looking back at how %rbp is filled we seem to be missing a shift since the excluded bit is at bit 15: ;; B7: # out( B10 B8 ) <- in( B6 ) Freq: 0.05 0x0000705314db36b7: movzwl 0x38(%rbx),%r11d 0x0000705314db36bc: mov %r11d,%ebp 0x0000705314db36bf: and $0x8000,%ebp
18-11-2024
Looks like a preexistent JFR bug. I can reproduce it using ReentrantLock even before JDK-8338383: diff --git a/test/jdk/jdk/jfr/jvm/LatchedThread.java b/test/jdk/jdk/jfr/jvm/LatchedThread.java index 42d31889dfa..5779cc8755c 100644 --- a/test/jdk/jdk/jfr/jvm/LatchedThread.java +++ b/test/jdk/jdk/jfr/jvm/LatchedThread.java @@ -24,12 +24,16 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.locks.ReentrantLock; +import java.util.concurrent.locks.Condition; public class LatchedThread implements Runnable { public final static ThreadGroup THREAD_GROUP = new ThreadGroup("Latched Threads"); public final Thread thread; private final CountDownLatch latch = new CountDownLatch(1); private final AtomicBoolean alive = new AtomicBoolean(true); + private final ReentrantLock rlock = new ReentrantLock(); + private final Condition cond = rlock.newCondition(); public LatchedThread(String name, boolean isVirtual) { if (isVirtual) { @@ -69,8 +73,11 @@ public Thread getThread() { public void stopAndJoin() throws InterruptedException { alive.set(false); - synchronized (alive) { - alive.notify(); + try { + rlock.lock(); + cond.signal(); + } finally { + rlock.unlock(); } join(); } @@ -79,11 +86,11 @@ public void run() { latch.countDown(); while (alive.get()) { try { - synchronized (alive) { - alive.wait(10); - } + rlock.lock(); + cond.awaitNanos(10 * 1000 * 1000); } catch (InterruptedException e) { - // ignore + } finally { + rlock.unlock(); } } }
17-11-2024
Seem, virtual thread event exclusion mechanism doesn't work. The events shouldn't be sent.
17-11-2024
When this test run with default VM option, the events which return from 'Events.fromRecording(recording)' is empty, so this test run passed with default VM option, because the core check verifyThreadExclusion of this test has nothing to do. When this tets run with -Xcomp vm option, the events which return from 'Events.fromRecording(recording)' has several entries, and the first event call 'event.getThread().getJavaThreadId()' report '"jdk.jfr.consumer.RecordedThread.getJavaThreadId()" because the return value of "jdk.jfr.consumer.RecordedEvent.getThread()" is null'. So, maybe this fails unreleated to C2 compiler.
17-11-2024
Removed os/arch/env. The problem is not specific to any os/arch.
16-11-2024
Confirming, it is regression after JDK-8338383
16-11-2024
Verified, that test fails with Xcomp and C2 only. So moving to compiler for further investigation and triaging.
16-11-2024
Maybe caused by JDK-8338383
15-11-2024

Relates :	JDK-8338383 - Implement JEP 491: Synchronize Virtual Threads without Pinning
Relates :	JDK-8318098 - Update jfr tests to replace keyword jfr with vm.flagless
Relates :	JDK-8284161 - Implementation of Virtual Threads (Preview)