On Tue, 25 Oct 2022 11:39:37 GMT, Roman Kennke <rken...@openjdk.org> wrote:
> There are several users and even mostly-identical implementations of > Threads::owning_thread_from_monitor_owner(), which I would like to > consolidate a little in preparation of JDK-8291555: > - JvmtiEnvBase::get_monitor_usage(): As the comment in > ObjectSynchronizer::get_lock_owner() suggests, the JVMTI code should call the > ObjectSynchronizer method. The only real difference is that JVMTI loads the > object header directly while OS spins to avoid INFLATING. This is harmless, > because JVMTI calls from safepoint, where INFLATING does not occur, and would > just do a simple load of the header. A little care must be taken to fetch the > monitor if exists a few lines below, to fill in monitor info. > - Two ThreadService methods call Threads::owning_thread_from_monitor_owner(), > but always only ever from a monitor. I would like to extract that special > case because with fast-locking this can be treated differently (with > fast-locking, monitor owners can only be JavaThread* or 'anonynmous'). It's > also a little cleaner IMO. > > Testing: > - [x] GHA (x86 and x-compile failures look like infra glitch) > - [x] tier1 > - [x] tier2 > - [x] tier3 > - [x] tier4 I have a concern about the new `has_owner()` function and whether it might cause problems with dead lock detection when an ObjectMonitor is being deflated. src/hotspot/share/runtime/objectMonitor.inline.hpp line 62: > 60: void* owner = owner_raw(); > 61: return owner != NULL || owner == DEFLATER_MARKER; > 62: } Why does has_owner() return `true` when `owner == DEFLATER_MARKER`? I'm only seeing one caller to the new `has_owner()` function in `ThreadService::find_deadlocks_at_safepoint()` and I don't understand why that code needs to think `has_owner()` needs to be `true` if the target ObjectMonitor is being deflated. That new `has_owner()` call will result in calling `Threads::owning_thread_from_monitor()` with `waitingToLockMonitor` which is being deflated. So the return from `Threads::owning_thread_from_monitor()` will be `NULL` which will result in us taking the `num_deadlocks++` code path. If I'm reading this right, then we'll report a deflating monitor as being in a deadlock. What am I missing here? ------------- Changes requested by dcubed (Reviewer). PR: https://git.openjdk.org/jdk/pull/10849
