On Mon, 27 Mar 2023 18:47:31 GMT, Roman Kennke <rken...@openjdk.org> wrote:
> > > > > > @rkennke Question about ZGC and LockStack::contains(): how does > > > > > > this work with colored pointers? Don't we have to mask the color > > > > > > bits out somehow when comparing? E.g. using `ZAddress::offset()` ? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > That would be a question for @fisk and/or @stefank. AFAIK, the color > > > > > bits should be masked by ZGC barriers _before_ the oops enter the > > > > > synchronization subsystem. But I kinda suspect that we are somehow > > > > > triggering a ZGC bug here. Maybe we require barriers when reading > > > > > oops from the lock-stack too? > > > > > > > > > > > > > > > > > > > > > > > > > > Oops that are processed in Thread::oops_do should not have load > > > > barriers. Other oops should have load barriers. > > > > > > > > > > > > Ok, good. The lockstack is processed in JavaThread::oops_do_no_frames() > > > which is called from Thread::oops_do(). But help me here: I believe ZGC > > > processes this stuff concurrently, right? So there might be a window > > > where the lock-stack oops would be unprocessed. The lock-stack would not > > > go under the stack-watermark machinery. And if some code (like JVMTI > > > deadlock detection pause) inspects the lockstack, it might see invalid > > > oops? Is that a plausible scenario, or am I missing something? > > > > > > The JVMTI deadlock detection runs in a safepoint, doesn't it? Safepoints > > call start_processing on all threads in safepoint cleanup for non-GC > > safepoints. That means the lock stack oops should have been processed when > > the deadlock detection logic runs in a safepoint. > > > > There appears to be a single code-path that inspects the lock-stack (and also > the usual stack under non-fast-locking) outside of safepoints: > > > > V [libjvm.so+0x180abd4] Threads::owning_thread_from_monitor(ThreadsList*, > ObjectMonitor*)+0x54 (threads.cpp:1433) > > V [libjvm.so+0x17a4bfc] ObjectSynchronizer::get_lock_owner(ThreadsList*, > Handle)+0x9c (synchronizer.cpp:1109) > > V [libjvm.so+0x1802db0] ThreadSnapshot::initialize(ThreadsList*, > JavaThread*)+0x270 (threadService.cpp:942) > > V [libjvm.so+0x1803244] > ThreadDumpResult::add_thread_snapshot(JavaThread*)+0x5c > (threadService.cpp:567) > > V [libjvm.so+0x12a0f64] jmm_GetThreadInfo+0x480 (management.cpp:1136) > > j > sun.management.ThreadImpl.getThreadInfo1([JI[Ljava/lang/management/ThreadInfo;)V+0 > java.management@21-internal > > > > Curiously, this seems to be in JMX code, which is also roughly where the > failure happens. I came across this code a couple of times and couldn't > really tell if it is safe to do that outside of a safepoint. In doubt I have > to assume it is not, and maybe this is the source of the failure? WDYT? Could be. When not running a handshake or safepoint, you need to call start_processing manually on the target thread, which will ensure the oops are fixed until the next safepoint poll. ------------- PR Comment: https://git.openjdk.org/jdk/pull/10907#issuecomment-1485752396