On Sat, 16 Nov 2024 05:21:40 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:
>> Hi
>> Could you please review the the fix that add locks information into SA jhsdb 
>> stack --mixed output.
>> 
>> Here is the motivation for this rfe and explanation why I add it into SA now.
>> 
>> The information about current owners of Hotspot Mutex is often very useful 
>> for dealock investigations.
>> 
>> The jcmd usually doesn't work because VM can't reach or exit safepoints. So 
>> it doesn't respond to jcmd.
>> 
>> The SA 'jstack --mixed' provides information about stacktraces on Java and 
>> non-Java Threads. So having information about locks along with stack traces 
>> might significantly help to identify issues.
>> 
>> The gdb allows to provide stacktraces, but the debug symbols are required to 
>> get info about locks. These symbols are often absent during execution.
>> Also the debugger solution is OS specifc.
>> 
>> The significant part of fix is refacotorrng of mutex_array to be vmStructs 
>> compatible.
>> 
>> The adding support of non-JavaThreads into SA might be implemented later to 
>> obtain more info about their names.
>> The example of output:
>> 
>> [2024-11-06T21:32:48.897414435Z] Gathering output for process 1620563
>> Attaching to process ID 1620533, please wait...
>> Debugger attached successfully.
>> Server compiler detected.
>> JVM version is 24-internal-adhoc.lmesnik.open
>> Deadlock Detection:
>> 
>> No deadlocks found.
>> 
>> Internal VM Mutex Threads_lock is owned by Unknnown thread (Might be 
>> non-Java Thread) with address: 0x00007f8cf825b190
>> Internal VM Mutex Compile_lock is owned by LockerThread with address: 
>> 0x00007f8cf8309a00
>> ----------------- 1620559 -----------------
>> "C1 CompilerThread4" https://github.com/openjdk/jdk/pull/28 daemon prio=9 
>> tid=0x00007f8c300566a0 nid=1620559 runnable [0x0000000000000000]
>> java.lang.Thread.State: RUNNABLE
>> JavaThread state: _thread_blocked
>> 0x00007f8cff11e88d syscall + 0x1d
>> 0x00007f8cfe6c99de LinuxWaitBarrier::wait(int) + 0x8e
>> 0x00007f8cfe2be409 SafepointMechanism::process(JavaThread*, bool, bool) + 
>> 0x79
>> 0x00007f8cfd53ea91 ThreadInVMfromNative::ThreadInVMfromNative(JavaThread*) + 
>> 0xc1
>> 0x00007f8cfd534a00 ciEnv::cache_jvmti_state() + 0x30
>> 0x00007f8cfd679614 CompileBroker::invoke_compiler_on_method(CompileTask*) + 
>> 0x204
>> 0x00007f8cfd67adc8 CompileBroker::compiler_thread_loop() + 0x5c8
>> 0x00007f8cfdb4426c JavaThread::thread_main_inner() + 0xcc
>> 0x00007f8cfe5a0bbe Thread::call_run() + 0xbe
>> 0x00007f8cfe16813b thread_native_entry(Thread*) + 0x12b
>> .....
>> ----------------- 1620554 -----------------
>> "LockerThread" https://...
>
> Leonid Mesnik has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   make test more robust

Just a couple of drive-by comments/suggestions.

The functionality seems like a reasonable addition.

Thanks

src/hotspot/share/prims/whitebox.cpp line 190:

> 188: WB_END
> 189: 
> 190: WB_ENTRY(jint, WB_LockAndStuckInSafepoint(JNIEnv* env, jobject wb))

Suggestion:

WB_ENTRY(jint, WB_TakeLockAndHangInSafepoint(JNIEnv* env, jobject wb))

src/hotspot/share/runtime/vmOperations.hpp line 64:

> 62: 
> 63: // used by whitebox API to emulate VM issues
> 64: // when VM can't operate and doesn't respond to jcm

Suggestion:

// when VM can't operate and doesn't respond to jcmd

src/hotspot/share/runtime/vmOperations.hpp line 65:

> 63: // used by whitebox API to emulate VM issues
> 64: // when VM can't operate and doesn't respond to jcm
> 65: class VM_ForceSafepointStuck: public VM_Operation {

Suggestion:

class VM_HangInSafepoint: public VM_Operation {

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/Mutex.java line 70:

> 68:   public String name() { return 
> CStringUtilities.getString(addr.getAddressAt(nameFieldOffset)); }
> 69: 
> 70:   public Address owner() { return addr.getAddressAt(ownerFieldOffset); }

Printing the `osThread()->thread_id()` might be more useful than printing the 
address.

-------------

PR Review: https://git.openjdk.org/jdk/pull/22171#pullrequestreview-2441565041
PR Review Comment: https://git.openjdk.org/jdk/pull/22171#discussion_r1845903441
PR Review Comment: https://git.openjdk.org/jdk/pull/22171#discussion_r1845900385
PR Review Comment: https://git.openjdk.org/jdk/pull/22171#discussion_r1845900921
PR Review Comment: https://git.openjdk.org/jdk/pull/22171#discussion_r1845902671

Reply via email to