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.

Sign up for GitHub

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

8334215: serviceability/dcmd/thread/PrintMountedVirtualThread.java failing with JTREG_TEST_THREAD_FACTORY=Virtual #19744

Closed
wants to merge 4 commits into from
+13 −11
Closed

8334215: serviceability/dcmd/thread/PrintMountedVirtualThread.java failing with JTREG_TEST_THREAD_FACTORY=Virtual #19744

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
63a8cb6
8334215: Fix PrintMountVThread if FACTORY=Virtual
txominpelu Jun 17, 2024
9a954ef
Improve assert and document conditional print
txominpelu Jun 17, 2024
05c0f75
Move the explanation before the if
txominpelu Jun 18, 2024
af85b26
Remove thread name from the output
txominpelu Jun 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/hotspot/share/runtime/threads.cpp
View file
Original file line number Diff line number Diff line change
@@ -1332,8 +1332,8 @@ void Threads::print_on(outputStream* st, bool print_stacks,
if (p->is_vthread_mounted()) {
const oop vt = p->vthread();
assert(vt != nullptr, "vthread should not be null when vthread is mounted");
// JavaThread._vthread can refer to the carrier thread. Print only if _vthread refers to a virtual thread.
if (vt != thread_oop) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a comment explaining why we have to check for this as without knowing the implementation quirks it seems non-sensical for a thread to have mounted itself!

Copy link
Contributor

@AlanBateman AlanBateman Jun 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a comment explaining why we have to check for this as without knowing the implementation quirks it seems non-sensical for a thread to have mounted itself!

Just to note that JavaThread._vthread is always the "current thread". It's the virtual thread when mounted, the carrier (as in itself) when there is no virtual thread mounted. It's done this way so that the C2 instrinic for currentThread doesn't need a branch/test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't aware of that, and I was going to suggest to add that information as documentation, but it seems to be already documented here:

jdk/src/hotspot/share/runtime/javaThread.hpp

Line 95 in ad0b54d

OopHandle _vthread; // the value returned by Thread.currentThread(): the virtual thread, if mounted, otherwise _threadObj

Copy link
Contributor Author

@txominpelu txominpelu Jun 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've still clarified why the if is required in 9a954ef

// JavaThread._vthread can refer to the carrier thread. Print only if _vthread refers to a virtual thread.
st->print_cr(" Mounted virtual thread \"%s\" #" INT64_FORMAT, JavaThread::name_for(vt), (int64_t)java_lang_Thread::thread_id(vt));
p->print_vthread_stack_on(st);
}