Skip to content

Commit

Permalink
8322237: Heap dump contains duplicate thread records for mounted virt…
Browse files Browse the repository at this point in the history
…ual threads

Reviewed-by: dholmes, sspitsyn
  • Loading branch information
Alex Menkov committed Jan 9, 2024
1 parent ee98d26 commit dd8ae61
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 2 deletions.
18 changes: 17 additions & 1 deletion src/hotspot/share/services/heapDumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1641,6 +1641,19 @@ class ThreadDumper : public CHeapObj<mtInternal> {
&& java_lang_VirtualThread::state(vt) != java_lang_VirtualThread::TERMINATED;
}

static bool is_vthread_mounted(oop vt) {
// The code should be consistent with the "mounted virtual thread" case
// (VM_HeapDumper::dump_stack_traces(), ThreadDumper::get_top_frame()).
// I.e. virtual thread is mounted if its carrierThread is not null
// and is_vthread_mounted() for the carrier thread returns true.
oop carrier_thread = java_lang_VirtualThread::carrier_thread(vt);
if (carrier_thread == nullptr) {
return false;
}
JavaThread* java_thread = java_lang_Thread::thread(carrier_thread);
return java_thread->is_vthread_mounted();
}

ThreadDumper(ThreadType thread_type, JavaThread* java_thread, oop thread_oop);

// affects frame_count
Expand Down Expand Up @@ -1918,7 +1931,10 @@ void HeapObjectDumper::do_object(oop o) {
if (o->is_instance()) {
// create a HPROF_GC_INSTANCE record for each object
DumperSupport::dump_instance(writer(), o, &_class_cache);
if (java_lang_VirtualThread::is_instance(o) && ThreadDumper::should_dump_vthread(o)) {
// If we encounter an unmounted virtual thread it needs to be dumped explicitly
// (mounted virtual threads are dumped with their carriers).
if (java_lang_VirtualThread::is_instance(o)
&& ThreadDumper::should_dump_vthread(o) && !ThreadDumper::is_vthread_mounted(o)) {
_vthread_dumper->dump_vthread(o, writer());
}
} else if (o->is_objArray()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;

Expand Down Expand Up @@ -223,11 +225,22 @@ private static void verifyDump(File dumpFile) throws Exception {
// Log all threads with stack traces and stack references.
List<ThreadObject> threads = snapshot.getThreads();
List<Root> roots = Collections.list(snapshot.getRoots());
// And detect thread object duplicates.
Set<Long> uniqueThreads = new HashSet<>();

log("Threads:");
for (ThreadObject thread: threads) {
JavaHeapObject threadObj = snapshot.findThing(thread.getId());
JavaClass threadClass = threadObj.getClazz();
StackTrace st = thread.getStackTrace();
StackFrame[] frames = st.getFrames();
log("thread " + thread.getIdString() + ", " + frames.length + " frames");
log("thread " + thread.getIdString() + " (" + threadClass.getName() + "), " + frames.length + " frames");

if (uniqueThreads.contains(thread.getId())) {
log(" - ERROR: duplicate");
} else {
uniqueThreads.add(thread.getId());
}

List<Root> stackRoots = findStackRoot(roots, thread);
for (int i = 0; i < frames.length; i++) {
Expand All @@ -250,6 +263,10 @@ private static void verifyDump(File dumpFile) throws Exception {
}
}

if (threads.size() != uniqueThreads.size()) {
throw new RuntimeException("Thread duplicates detected (" + (threads.size() - uniqueThreads.size()) + ")");
}

// Verify objects from thread stacks are dumped.
test(snapshot, VThreadInHeapDumpTarg.VThreadMountedReferenced.class);
test(snapshot, VThreadInHeapDumpTarg.PThreadReferenced.class);
Expand Down

1 comment on commit dd8ae61

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.