RFR: 8247729: GetObjectMonitorUsage() might return inconsistent information

serguei.spitsyn at oracle.com serguei.spitsyn at oracle.com
Fri Jun 19 06:25:06 UTC 2020


Hi Yasumasa,

Looks good.

Thanks,
Serguei


On 6/18/20 18:56, Yasumasa Suenaga wrote:
> Hi Serguei,
>
> I tested vmTestbase/nsk/jdi with webrev.03, all tests work fine on my 
> laptop.
>
>   http://cr.openjdk.java.net/~ysuenaga/JDK-8247729/webrev.03/
>
>
> Thanks,
>
> Yasumasa
>
>
> On 2020/06/19 9:22, serguei.spitsyn at oracle.com wrote:
>> Hi Yasumasa,
>>
>> It would be even more safe to run the JDI tests as well.
>> The ObjectReference owningThread(), waitingThreads() and entryCount() 
>> are based on this JVMTI function.
>> See: 
>> https://docs.oracle.com/en/java/javase/14/docs/api/jdk.jdi/com/sun/jdi/ObjectReference.html
>>
>> Thanks,
>> Serguei
>>
>>
>> On 6/18/20 17:01, Yasumasa Suenaga wrote:
>>> Thanks Serguei!
>>>
>>> I fixed them, and the change works fine on my laptop with 
>>> serviceability/jvmti and vmTestbase/nsk/jvmti on Linux x64.
>>> I will push it later.
>>>
>>>
>>> Yasumasa
>>>
>>>
>>> On 2020/06/19 6:42, serguei.spitsyn at oracle.com wrote:
>>>> Hi Yasumasa,
>>>>
>>>> This looks good, nice simplification.
>>>>
>>>> A couple of minor comments.
>>>>
>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8247729/webrev.02/src/hotspot/share/prims/jvmtiEnvBase.cpp.frames.html 
>>>>
>>>>
>>>> 995 ThreadsListHandle tlh(current_thread); 1052 ThreadsListHandle 
>>>> tlh(current_thread);
>>>>
>>>> We can share one tlh for both fragments.
>>>>
>>>> 942 HandleMark hm; 1051 HandleMark hm;
>>>>
>>>> The second HandleMark is not needed.
>>>> Also, we can use current_thread in the first one:
>>>>
>>>> HandleMark hm(current_thread);
>>>>
>>>>
>>>> I do not need to see another webrev if you fix the above.
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>> On 6/18/20 07:56, Yasumasa Suenaga wrote:
>>>>> Hi Daniel,
>>>>>
>>>>> On 2020/06/18 23:38, Daniel D. Daugherty wrote:
>>>>>> On 6/18/20 5:07 AM, Yasumasa Suenaga wrote:
>>>>>>> On 2020/06/18 17:36, David Holmes wrote:
>>>>>>>> On 18/06/2020 3:47 pm, Yasumasa Suenaga wrote:
>>>>>>>>> Hi David,
>>>>>>>>>
>>>>>>>>> Both ThreadsListHandle and ResourceMarks would use 
>>>>>>>>> `Thread::current()` for their resource. It is set as default 
>>>>>>>>> parameter in c'tor.
>>>>>>>>> Do you mean we should it explicitly in c'tor?
>>>>>>>>
>>>>>>>> Yes pass current_thread so we don't do the additional 
>>>>>>>> unnecessary calls to Thread::current().
>>>>>>>
>>>>>>> Ok, I've fixed them. Could you review again?
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8247729/webrev.02/
>>>>>>
>>>>>> src/hotspot/share/prims/jvmtiEnv.cpp
>>>>>>      L2842:   // It need to perform at safepoint for gathering 
>>>>>> stable data
>>>>>>          Perhaps:
>>>>>>               // This needs to be performed at a safepoint to 
>>>>>> gather stable data
>>>>>
>>>>> I will change it before pushing.
>>>>>
>>>>>
>>>>>> src/hotspot/share/prims/jvmtiEnvBase.cpp
>>>>>>      No comments.
>>>>>>
>>>>>> Thumbs up.
>>>>>>
>>>>>> What testing has been done on this fix?
>>>>>
>>>>> I tested this change on serviceability/jvmti and 
>>>>> vmTestbase/nsk/jvmti on Linux x64.
>>>>>
>>>>>
>>>>>> Also, please wait to hear from Serguei on this fix...
>>>>>
>>>>> Ok.
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>>> Dan
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>>> David
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2020/06/18 13:58, David Holmes wrote:
>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>
>>>>>>>>>> On 18/06/2020 12:59 pm, Yasumasa Suenaga wrote:
>>>>>>>>>>> Hi Serguei,
>>>>>>>>>>>
>>>>>>>>>>> Thanks for your comment!
>>>>>>>>>>> I uploaded new webrev:
>>>>>>>>>>>
>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8247729/webrev.01/
>>>>>>>>>>>
>>>>>>>>>>> I'm not sure the following change is correct.
>>>>>>>>>>> Can we assume owning_thread is not NULL at safepoint?
>>>>>>>>>>
>>>>>>>>>> We can if "owner != NULL". So that change seem fine to me.
>>>>>>>>>>
>>>>>>>>>> But given this is now only executed at a safepoint there are 
>>>>>>>>>> additional simplifications that can be made:
>>>>>>>>>>
>>>>>>>>>> - current thread determination can be simplified:
>>>>>>>>>>
>>>>>>>>>> 945   Thread* current_thread = Thread::current();
>>>>>>>>>>
>>>>>>>>>> becomes:
>>>>>>>>>>
>>>>>>>>>>     Thread* current_thread = VMThread::vm_thread();
>>>>>>>>>>     assert(current_thread == Thread::current(), "must be");
>>>>>>>>>>
>>>>>>>>>> - these comments can be removed
>>>>>>>>>>
>>>>>>>>>>   994       // Use current thread since function can be 
>>>>>>>>>> called from a
>>>>>>>>>>   995       // JavaThread or the VMThread.
>>>>>>>>>> 1053       // Use current thread since function can be called 
>>>>>>>>>> from a
>>>>>>>>>> 1054       // JavaThread or the VMThread.
>>>>>>>>>>
>>>>>>>>>> - these TLH constructions should be passing current_thread 
>>>>>>>>>> (existing bug)
>>>>>>>>>>
>>>>>>>>>> 996       ThreadsListHandle tlh;
>>>>>>>>>> 1055       ThreadsListHandle tlh;
>>>>>>>>>>
>>>>>>>>>> - All ResourceMarks should be passing current_thread 
>>>>>>>>>> (existing bug)
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Aside: there is a major inconsistency between the spec and 
>>>>>>>>>> implementation for this method. I've traced the history to 
>>>>>>>>>> see how this came about from JVMDI (ref JDK-4546581) but it 
>>>>>>>>>> never resulted in the JVM TI specification clearly stating 
>>>>>>>>>> what the waiters/waiter_count means. I will file a bug to 
>>>>>>>>>> have the spec clarified to match the implementation (even 
>>>>>>>>>> though I think the implementation is what is wrong). :(
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> David
>>>>>>>>>> -----
>>>>>>>>>>
>>>>>>>>>>> All tests on submit repo and serviceability/jvmti and 
>>>>>>>>>>> vmTestbase/nsk/jvmti have been passed with this change.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> ```
>>>>>>>>>>>         // This monitor is owned so we have to find the 
>>>>>>>>>>> owning JavaThread.
>>>>>>>>>>>         owning_thread = 
>>>>>>>>>>> Threads::owning_thread_from_monitor_owner(tlh.list(), owner);
>>>>>>>>>>> -      // Cannot assume (owning_thread != NULL) here because 
>>>>>>>>>>> this function
>>>>>>>>>>> -      // may not have been called at a safepoint and the 
>>>>>>>>>>> owning_thread
>>>>>>>>>>> -      // might not be suspended.
>>>>>>>>>>> -      if (owning_thread != NULL) {
>>>>>>>>>>> -        // The monitor's owner either has to be the current 
>>>>>>>>>>> thread, at safepoint
>>>>>>>>>>> -        // or it has to be suspended. Any of these 
>>>>>>>>>>> conditions will prevent both
>>>>>>>>>>> -        // contending and waiting threads from modifying 
>>>>>>>>>>> the state of
>>>>>>>>>>> -        // the monitor.
>>>>>>>>>>> -        if (!at_safepoint && 
>>>>>>>>>>> !owning_thread->is_thread_fully_suspended(true, &debug_bits)) {
>>>>>>>>>>> -          // Don't worry! This return of 
>>>>>>>>>>> JVMTI_ERROR_THREAD_NOT_SUSPENDED
>>>>>>>>>>> -          // will not make it back to the JVM/TI agent. The 
>>>>>>>>>>> error code will
>>>>>>>>>>> -          // get intercepted in 
>>>>>>>>>>> JvmtiEnv::GetObjectMonitorUsage() which
>>>>>>>>>>> -          // will retry the call via a 
>>>>>>>>>>> VM_GetObjectMonitorUsage VM op.
>>>>>>>>>>> -          return JVMTI_ERROR_THREAD_NOT_SUSPENDED;
>>>>>>>>>>> -        }
>>>>>>>>>>> -        HandleMark hm;
>>>>>>>>>>> +      assert(owning_thread != NULL, "owning JavaThread must 
>>>>>>>>>>> not be NULL");
>>>>>>>>>>>           Handle     th(current_thread, 
>>>>>>>>>>> owning_thread->threadObj());
>>>>>>>>>>>           ret.owner = (jthread)jni_reference(calling_thread, 
>>>>>>>>>>> th);
>>>>>>>>>>>
>>>>>>>>>>> ```
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>>
>>>>>>>>>>> Yasumasa
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 2020/06/18 0:42, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>
>>>>>>>>>>>> This fix is not enough.
>>>>>>>>>>>> The function JvmtiEnvBase::get_object_monitor_usage works 
>>>>>>>>>>>> in two modes: in VMop and non-VMop.
>>>>>>>>>>>> The non-VMop mode has to be removed.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Serguei
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On 6/17/20 02:18, Yasumasa Suenaga wrote:
>>>>>>>>>>>>> (Change subject for RFR)
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>> I filed it to JBS and upload a webrev for it.
>>>>>>>>>>>>> Could you review it?
>>>>>>>>>>>>>
>>>>>>>>>>>>>   JBS: https://bugs.openjdk.java.net/browse/JDK-8247729
>>>>>>>>>>>>>   webrev: 
>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8247729/webrev.00/
>>>>>>>>>>>>>
>>>>>>>>>>>>> This change has passed tests on submit repo.
>>>>>>>>>>>>> Also I tested it with serviceability/jvmti and 
>>>>>>>>>>>>> vmTestbase/nsk/jvmti on Linux x64.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 2020/06/17 14:37, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>>>>>> Yes. It seems we have a consensus.
>>>>>>>>>>>>>> Thank you for taking care about it.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Serguei
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 6/16/20 18:34, David Holmes wrote:
>>>>>>>>>>>>>>>> Ok, may I file it to JBS and fix it? 
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Go for it! :)
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 17/06/2020 10:23 am, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>> On 2020/06/17 8:47, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>>>>>>>>> Hi Dan, David and Yasumasa,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On 6/16/20 07:39, Daniel D. Daugherty wrote:
>>>>>>>>>>>>>>>>>> On 6/15/20 9:28 PM, David Holmes wrote:
>>>>>>>>>>>>>>>>>>> On 16/06/2020 10:57 am, Daniel D. Daugherty wrote:
>>>>>>>>>>>>>>>>>>>> On 6/15/20 7:19 PM, David Holmes wrote:
>>>>>>>>>>>>>>>>>>>>> On 16/06/2020 8:40 am, Daniel D. Daugherty wrote:
>>>>>>>>>>>>>>>>>>>>>> On 6/15/20 6:14 PM, David Holmes wrote:
>>>>>>>>>>>>>>>>>>>>>>> Hi Dan,
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> On 15/06/2020 11:38 pm, Daniel D. Daugherty wrote:
>>>>>>>>>>>>>>>>>>>>>>>> On 6/15/20 3:26 AM, David Holmes wrote:
>>>>>>>>>>>>>>>>>>>>>>>>> On 15/06/2020 4:02 pm, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>> Hi David,
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> On 2020/06/15 14:15, David Holmes wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> On 15/06/2020 2:49 pm, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> I wonder why 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> JvmtiEnvBase::get_object_monitor_usage() 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> (implementation of GetObjectMonitorUsage()) 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> does not perform at safepoint.
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> GetObjectMonitorUsage will use a safepoint 
>>>>>>>>>>>>>>>>>>>>>>>>>>> if the target is not suspended:
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> jvmtiError
>>>>>>>>>>>>>>>>>>>>>>>>>>> JvmtiEnv::GetObjectMonitorUsage(jobject 
>>>>>>>>>>>>>>>>>>>>>>>>>>> object, jvmtiMonitorUsage* info_ptr) {
>>>>>>>>>>>>>>>>>>>>>>>>>>>    JavaThread* calling_thread = 
>>>>>>>>>>>>>>>>>>>>>>>>>>> JavaThread::current();
>>>>>>>>>>>>>>>>>>>>>>>>>>>    jvmtiError err = 
>>>>>>>>>>>>>>>>>>>>>>>>>>> get_object_monitor_usage(calling_thread, 
>>>>>>>>>>>>>>>>>>>>>>>>>>> object, info_ptr);
>>>>>>>>>>>>>>>>>>>>>>>>>>>    if (err == 
>>>>>>>>>>>>>>>>>>>>>>>>>>> JVMTI_ERROR_THREAD_NOT_SUSPENDED) {
>>>>>>>>>>>>>>>>>>>>>>>>>>>      // Some of the critical threads were 
>>>>>>>>>>>>>>>>>>>>>>>>>>> not suspended. go to a safepoint and try again
>>>>>>>>>>>>>>>>>>>>>>>>>>> VM_GetObjectMonitorUsage op(this, 
>>>>>>>>>>>>>>>>>>>>>>>>>>> calling_thread, object, info_ptr);
>>>>>>>>>>>>>>>>>>>>>>>>>>> VMThread::execute(&op);
>>>>>>>>>>>>>>>>>>>>>>>>>>>      err = op.result();
>>>>>>>>>>>>>>>>>>>>>>>>>>>    }
>>>>>>>>>>>>>>>>>>>>>>>>>>>    return err;
>>>>>>>>>>>>>>>>>>>>>>>>>>> } /* end GetObject */
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> I saw this code, so I guess there are some 
>>>>>>>>>>>>>>>>>>>>>>>>>> cases when JVMTI_ERROR_THREAD_NOT_SUSPENDED 
>>>>>>>>>>>>>>>>>>>>>>>>>> is not returned from get_object_monitor_usage().
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Monitor owner would be acquired from 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> monitor object at first [1], but it would 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> perform concurrently.
>>>>>>>>>>>>>>>>>>>>>>>>>>>> If owner thread is not suspended, the owner 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> might be changed to others in subsequent code.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> For example, the owner might release the 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> monitor before [2].
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> The expectation is that when we find an 
>>>>>>>>>>>>>>>>>>>>>>>>>>> owner thread it is either suspended or not. 
>>>>>>>>>>>>>>>>>>>>>>>>>>> If it is suspended then it cannot release 
>>>>>>>>>>>>>>>>>>>>>>>>>>> the monitor. If it is not suspended we 
>>>>>>>>>>>>>>>>>>>>>>>>>>> detect that and redo the whole query at a 
>>>>>>>>>>>>>>>>>>>>>>>>>>> safepoint.
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> I think the owner thread might resume 
>>>>>>>>>>>>>>>>>>>>>>>>>> unfortunately after suspending check.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> Yes you are right. I was thinking resuming 
>>>>>>>>>>>>>>>>>>>>>>>>> also required a safepoint but it only requires 
>>>>>>>>>>>>>>>>>>>>>>>>> the Threads_lock. So yes the code is wrong.
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Which code is wrong?
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Yes, a rogue resume can happen when the 
>>>>>>>>>>>>>>>>>>>>>>>> GetObjectMonitorUsage() caller
>>>>>>>>>>>>>>>>>>>>>>>> has started the process of gathering the 
>>>>>>>>>>>>>>>>>>>>>>>> information while not at a
>>>>>>>>>>>>>>>>>>>>>>>> safepoint. Thus the information returned by 
>>>>>>>>>>>>>>>>>>>>>>>> GetObjectMonitorUsage()
>>>>>>>>>>>>>>>>>>>>>>>> might be stale, but that's a bug in the agent 
>>>>>>>>>>>>>>>>>>>>>>>> code.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> The code tries to make sure that it either 
>>>>>>>>>>>>>>>>>>>>>>> collects data about a monitor owned by a thread 
>>>>>>>>>>>>>>>>>>>>>>> that is suspended, or else it collects that data 
>>>>>>>>>>>>>>>>>>>>>>> at a safepoint. But the owning thread can be 
>>>>>>>>>>>>>>>>>>>>>>> resumed just after the code determined it was 
>>>>>>>>>>>>>>>>>>>>>>> suspended. The monitor can then be released and 
>>>>>>>>>>>>>>>>>>>>>>> the information gathered not only stale but 
>>>>>>>>>>>>>>>>>>>>>>> potentially completely wrong as it could now be 
>>>>>>>>>>>>>>>>>>>>>>> owned by a different thread and will report that 
>>>>>>>>>>>>>>>>>>>>>>> thread's entry count.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> If the agent is not using SuspendThread(), then 
>>>>>>>>>>>>>>>>>>>>>> as soon as
>>>>>>>>>>>>>>>>>>>>>> GetObjectMonitorUsage() returns to the caller the 
>>>>>>>>>>>>>>>>>>>>>> information
>>>>>>>>>>>>>>>>>>>>>> can be stale. In fact as soon as the 
>>>>>>>>>>>>>>>>>>>>>> implementation returns
>>>>>>>>>>>>>>>>>>>>>> from the safepoint that gathered the info, the 
>>>>>>>>>>>>>>>>>>>>>> target thread
>>>>>>>>>>>>>>>>>>>>>> could have moved on.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> That isn't the issue. That the info is stale is 
>>>>>>>>>>>>>>>>>>>>> fine. But the expectation is that the information 
>>>>>>>>>>>>>>>>>>>>> was actually an accurate snapshot of the state of 
>>>>>>>>>>>>>>>>>>>>> the monitor at some point in time. The current 
>>>>>>>>>>>>>>>>>>>>> code does not ensure that.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Please explain. I clearly don't understand why you 
>>>>>>>>>>>>>>>>>>>> think the info
>>>>>>>>>>>>>>>>>>>> returned isn't "an accurate snapshot of the state 
>>>>>>>>>>>>>>>>>>>> of the monitor
>>>>>>>>>>>>>>>>>>>> at some point in time".
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Because it may not be a "snapshot" at all. There is 
>>>>>>>>>>>>>>>>>>> no atomicity**. The reported owner thread may not 
>>>>>>>>>>>>>>>>>>> own it any longer when the entry count is read, so 
>>>>>>>>>>>>>>>>>>> straight away you may have the wrong entry count 
>>>>>>>>>>>>>>>>>>> information. The set of threads trying to acquire 
>>>>>>>>>>>>>>>>>>> the monitor, or wait on the monitor can change in 
>>>>>>>>>>>>>>>>>>> unexpected ways. It would be possible for instance 
>>>>>>>>>>>>>>>>>>> to report the same thread as being the owner, being 
>>>>>>>>>>>>>>>>>>> blocked trying to enter the monitor, and being in 
>>>>>>>>>>>>>>>>>>> the wait-set of the monitor - apparently all at the 
>>>>>>>>>>>>>>>>>>> same time!
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> ** even if the owner is suspended we don't have 
>>>>>>>>>>>>>>>>>>> complete atomicity because threads can join the set 
>>>>>>>>>>>>>>>>>>> of threads trying to enter the monitor (unless they 
>>>>>>>>>>>>>>>>>>> are all suspended).
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Consider the case when the monitor's owner is _not_ 
>>>>>>>>>>>>>>>>>> suspended:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>   - GetObjectMonitorUsage() uses a safepoint to 
>>>>>>>>>>>>>>>>>> gather the info about
>>>>>>>>>>>>>>>>>>     the object's monitor. Since we're at a safepoint, 
>>>>>>>>>>>>>>>>>> the info that
>>>>>>>>>>>>>>>>>>     we are gathering cannot change until we return 
>>>>>>>>>>>>>>>>>> from the safepoint.
>>>>>>>>>>>>>>>>>>     It is a snapshot and a valid one at that.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Consider the case when the monitor's owner is suspended:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>   - GetObjectMonitorUsage() will gather info about 
>>>>>>>>>>>>>>>>>> the object's
>>>>>>>>>>>>>>>>>>     monitor while _not_ at a safepoint. Assuming that 
>>>>>>>>>>>>>>>>>> no other
>>>>>>>>>>>>>>>>>>     thread is suspended, then entry_count can change 
>>>>>>>>>>>>>>>>>> because
>>>>>>>>>>>>>>>>>>     another thread can block on entry while we are 
>>>>>>>>>>>>>>>>>> gathering
>>>>>>>>>>>>>>>>>>     info. waiter_count and waiters can change if a 
>>>>>>>>>>>>>>>>>> thread was
>>>>>>>>>>>>>>>>>>     in a timed wait that has timed out and now that 
>>>>>>>>>>>>>>>>>> thread is
>>>>>>>>>>>>>>>>>>     blocked on re-entry. I don't think that 
>>>>>>>>>>>>>>>>>> notify_waiter_count
>>>>>>>>>>>>>>>>>>     and notify_waiters can change.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>     So in this case, the owner info and notify info 
>>>>>>>>>>>>>>>>>> is stable,
>>>>>>>>>>>>>>>>>>     but the entry_count and waiter info is not stable.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Consider the case when the monitor is not owned:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>   - GetObjectMonitorUsage() will start to gather info 
>>>>>>>>>>>>>>>>>> about the
>>>>>>>>>>>>>>>>>>     object's monitor while _not_ at a safepoint. If 
>>>>>>>>>>>>>>>>>> it finds a
>>>>>>>>>>>>>>>>>>     thread on the entry queue that is not suspended, 
>>>>>>>>>>>>>>>>>> then it will
>>>>>>>>>>>>>>>>>>     bail out and redo the info gather at a safepoint. 
>>>>>>>>>>>>>>>>>> I just
>>>>>>>>>>>>>>>>>>     noticed that it doesn't check for suspension for 
>>>>>>>>>>>>>>>>>> the threads
>>>>>>>>>>>>>>>>>>     on the waiters list so a timed Object.wait() call 
>>>>>>>>>>>>>>>>>> can cause
>>>>>>>>>>>>>>>>>>     some confusion here.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>     So in this case, the owner info is not stable if 
>>>>>>>>>>>>>>>>>> a thread
>>>>>>>>>>>>>>>>>>     comes out of a timed wait and reenters the 
>>>>>>>>>>>>>>>>>> monitor. This
>>>>>>>>>>>>>>>>>>     case is no different than if a "barger" thread 
>>>>>>>>>>>>>>>>>> comes in
>>>>>>>>>>>>>>>>>>     after the NULL owner field is observed and enters 
>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>     monitor. We'll return that there is no owner, a 
>>>>>>>>>>>>>>>>>> list of
>>>>>>>>>>>>>>>>>>     suspended pending entry thread and a list of waiting
>>>>>>>>>>>>>>>>>>     threads. The reality is that the object's monitor is
>>>>>>>>>>>>>>>>>>     owned by the "barger" that completely bypassed 
>>>>>>>>>>>>>>>>>> the entry
>>>>>>>>>>>>>>>>>>     queue by virtue of seeing the NULL owner field at 
>>>>>>>>>>>>>>>>>> exactly
>>>>>>>>>>>>>>>>>>     the right time.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> So the owner field is only stable when we have an 
>>>>>>>>>>>>>>>>>> owner. If
>>>>>>>>>>>>>>>>>> that owner is not suspended, then the other fields 
>>>>>>>>>>>>>>>>>> are also
>>>>>>>>>>>>>>>>>> stable because we gathered the info at a safepoint. 
>>>>>>>>>>>>>>>>>> If the
>>>>>>>>>>>>>>>>>> owner is suspended, then the owner and notify info is 
>>>>>>>>>>>>>>>>>> stable,
>>>>>>>>>>>>>>>>>> but the entry_count and waiter info is not stable.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> If we have a NULL owner field, then the info is only 
>>>>>>>>>>>>>>>>>> stable
>>>>>>>>>>>>>>>>>> if you have a non-suspended thread on the entry list. 
>>>>>>>>>>>>>>>>>> Ouch!
>>>>>>>>>>>>>>>>>> That's deterministic, but not without some work.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Okay so only when we gather the info at a safepoint 
>>>>>>>>>>>>>>>>>> is all
>>>>>>>>>>>>>>>>>> of it a valid and stable snapshot. Unfortunately, we 
>>>>>>>>>>>>>>>>>> only
>>>>>>>>>>>>>>>>>> do that at a safepoint when the owner thread is not 
>>>>>>>>>>>>>>>>>> suspended
>>>>>>>>>>>>>>>>>> or if owner == NULL and one of the entry threads is not
>>>>>>>>>>>>>>>>>> suspended. If either of those conditions is not true, 
>>>>>>>>>>>>>>>>>> then
>>>>>>>>>>>>>>>>>> the different pieces of info is unstable to varying 
>>>>>>>>>>>>>>>>>> degrees.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> As for this claim:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> It would be possible for instance to report the same 
>>>>>>>>>>>>>>>>>>> thread
>>>>>>>>>>>>>>>>>>> as being the owner, being blocked trying to enter 
>>>>>>>>>>>>>>>>>>> the monitor,
>>>>>>>>>>>>>>>>>>> and being in the wait-set of the monitor - 
>>>>>>>>>>>>>>>>>>> apparently all at
>>>>>>>>>>>>>>>>>>> the same time! 
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I can't figure out a way to make that scenario work. 
>>>>>>>>>>>>>>>>>> If the
>>>>>>>>>>>>>>>>>> thread is seen as the owner and is not suspended, 
>>>>>>>>>>>>>>>>>> then we
>>>>>>>>>>>>>>>>>> gather info at a safepoint. If it is suspended, then 
>>>>>>>>>>>>>>>>>> it can't
>>>>>>>>>>>>>>>>>> then be seen as on the entry queue or on the wait 
>>>>>>>>>>>>>>>>>> queue since
>>>>>>>>>>>>>>>>>> it is suspended. If it is seen on the entry queue and 
>>>>>>>>>>>>>>>>>> is not
>>>>>>>>>>>>>>>>>> suspended, then we gather info at a safepoint. If it is
>>>>>>>>>>>>>>>>>> suspended on the entry queue, then it can't be seen 
>>>>>>>>>>>>>>>>>> on the
>>>>>>>>>>>>>>>>>> wait queue.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> So the info instability of this API is bad, but it's not
>>>>>>>>>>>>>>>>>> quite that bad. :-) (That is a small mercy.)
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Handshaking is not going to make this situation any 
>>>>>>>>>>>>>>>>>> better
>>>>>>>>>>>>>>>>>> for GetObjectMonitorUsage(). If the monitor is owned 
>>>>>>>>>>>>>>>>>> and we
>>>>>>>>>>>>>>>>>> handshake with the owner, the stability or 
>>>>>>>>>>>>>>>>>> instability of
>>>>>>>>>>>>>>>>>> the other fields remains the same as when 
>>>>>>>>>>>>>>>>>> SuspendThread is
>>>>>>>>>>>>>>>>>> used. Handshaking with all threads won't make the 
>>>>>>>>>>>>>>>>>> data as
>>>>>>>>>>>>>>>>>> stable as when at a safepoint because individual threads
>>>>>>>>>>>>>>>>>> can resume execution after doing their handshake so 
>>>>>>>>>>>>>>>>>> there
>>>>>>>>>>>>>>>>>> will still be field instability.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Short version: GetObjectMonitorUsage() should only 
>>>>>>>>>>>>>>>>>> gather
>>>>>>>>>>>>>>>>>> data at a safepoint. Yes, I've changed my mind.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I agree with this.
>>>>>>>>>>>>>>>>> The advantages are:
>>>>>>>>>>>>>>>>>   - the result is stable
>>>>>>>>>>>>>>>>>   - the implementation can be simplified
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Performance impact is not very clear but should not be 
>>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>>> big as suspending all the threads has some overhead too.
>>>>>>>>>>>>>>>>> I'm not sure if using handshakes can make performance 
>>>>>>>>>>>>>>>>> better.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Ok, may I file it to JBS and fix it?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>> Serguei
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Dan
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>>>>>> -----
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> The only way to make sure you don't have stale 
>>>>>>>>>>>>>>>>>>>>>> information is
>>>>>>>>>>>>>>>>>>>>>> to use SuspendThread(), but it's not required. 
>>>>>>>>>>>>>>>>>>>>>> Perhaps the doc
>>>>>>>>>>>>>>>>>>>>>> should have more clear about the possibility of 
>>>>>>>>>>>>>>>>>>>>>> returning stale
>>>>>>>>>>>>>>>>>>>>>> info. That's a question for Robert F.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> GetObjectMonitorUsage says nothing about 
>>>>>>>>>>>>>>>>>>>>>>> thread's being suspended so I can't see how this 
>>>>>>>>>>>>>>>>>>>>>>> could be construed as an agent bug.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> In your scenario above, you mention that the 
>>>>>>>>>>>>>>>>>>>>>> target thread was
>>>>>>>>>>>>>>>>>>>>>> suspended, GetObjectMonitorUsage() was called 
>>>>>>>>>>>>>>>>>>>>>> while the target
>>>>>>>>>>>>>>>>>>>>>> was suspended, and then the target thread was 
>>>>>>>>>>>>>>>>>>>>>> resumed after
>>>>>>>>>>>>>>>>>>>>>> GetObjectMonitorUsage() checked for suspension, 
>>>>>>>>>>>>>>>>>>>>>> but before
>>>>>>>>>>>>>>>>>>>>>> GetObjectMonitorUsage() was able to gather the info.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> All three of those calls: SuspendThread(), 
>>>>>>>>>>>>>>>>>>>>>> GetObjectMonitorUsage()
>>>>>>>>>>>>>>>>>>>>>> and ResumeThread() are made by the agent and the 
>>>>>>>>>>>>>>>>>>>>>> agent should not
>>>>>>>>>>>>>>>>>>>>>> resume the target thread while also calling 
>>>>>>>>>>>>>>>>>>>>>> GetObjectMonitorUsage().
>>>>>>>>>>>>>>>>>>>>>> The calls were allowed to be made out of order so 
>>>>>>>>>>>>>>>>>>>>>> agent bug.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Perhaps. I was thinking more generally about an 
>>>>>>>>>>>>>>>>>>>>> independent resume, but you're right that doesn't 
>>>>>>>>>>>>>>>>>>>>> really make a lot of sense. But when the spec says 
>>>>>>>>>>>>>>>>>>>>> nothing about suspension ...
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> And it is intentional that suspension is not 
>>>>>>>>>>>>>>>>>>>> required. JVM/DI and JVM/PI
>>>>>>>>>>>>>>>>>>>> used to require suspension for these kinds of 
>>>>>>>>>>>>>>>>>>>> get-the-info APIs. JVM/TI
>>>>>>>>>>>>>>>>>>>> intentionally was designed to not require suspension.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> As I've said before, we could add a note about the 
>>>>>>>>>>>>>>>>>>>> data being potentially
>>>>>>>>>>>>>>>>>>>> stale unless SuspendThread is used. I think of it 
>>>>>>>>>>>>>>>>>>>> like stat(2). You can
>>>>>>>>>>>>>>>>>>>> fetch the file's info, but there's no guarantee 
>>>>>>>>>>>>>>>>>>>> that the info is current
>>>>>>>>>>>>>>>>>>>> by the time you process what you got back. Is it 
>>>>>>>>>>>>>>>>>>>> too much motherhood to
>>>>>>>>>>>>>>>>>>>> state that the data might be stale? I could go 
>>>>>>>>>>>>>>>>>>>> either way...
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Using a handshake on the owner thread will allow 
>>>>>>>>>>>>>>>>>>>>>>> this to be fixed in the future without 
>>>>>>>>>>>>>>>>>>>>>>> forcing/using any safepoints.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> I have to think about that which is why I'm 
>>>>>>>>>>>>>>>>>>>>>> avoiding talking about
>>>>>>>>>>>>>>>>>>>>>> handshakes in this thread.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Effectively the handshake can "suspend" the thread 
>>>>>>>>>>>>>>>>>>>>> whilst the monitor is queried. In effect the 
>>>>>>>>>>>>>>>>>>>>> operation would create a per-thread safepoint.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> I "know" that, but I still need time to think about 
>>>>>>>>>>>>>>>>>>>> it and probably
>>>>>>>>>>>>>>>>>>>> see the code to see if there are holes...
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Semantically it is no different to the code 
>>>>>>>>>>>>>>>>>>>>> actually suspending the owner thread, but it can't 
>>>>>>>>>>>>>>>>>>>>> actually do that because suspends/resume don't nest.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Yeah... we used have a suspend count back when we 
>>>>>>>>>>>>>>>>>>>> tracked internal and
>>>>>>>>>>>>>>>>>>>> external suspends separately. That was a nightmare...
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Dan
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> Dan
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Dan
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> JavaThread::is_ext_suspend_completed() is 
>>>>>>>>>>>>>>>>>>>>>>>>>> used to check thread state, it returns `true` 
>>>>>>>>>>>>>>>>>>>>>>>>>> when the thread is sleeping [3], or when it 
>>>>>>>>>>>>>>>>>>>>>>>>>> performs in native [4].
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> Sure but if the thread is actually suspended 
>>>>>>>>>>>>>>>>>>>>>>>>> it can't continue execution in the VM or in 
>>>>>>>>>>>>>>>>>>>>>>>>> Java code.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> This appears to be an optimisation for the 
>>>>>>>>>>>>>>>>>>>>>>>>>>> assumed common case where threads are first 
>>>>>>>>>>>>>>>>>>>>>>>>>>> suspended and then the monitors are queried.
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> I agree with this, but I could find out it 
>>>>>>>>>>>>>>>>>>>>>>>>>> from JVMTI spec - it just says "Get 
>>>>>>>>>>>>>>>>>>>>>>>>>> information about the object's monitor."
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> Yes it was just an implementation 
>>>>>>>>>>>>>>>>>>>>>>>>> optimisation, nothing to do with the spec.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> GetObjectMonitorUsage() might return 
>>>>>>>>>>>>>>>>>>>>>>>>>> incorrect information in some case.
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> It starts with finding owner thread, but the 
>>>>>>>>>>>>>>>>>>>>>>>>>> owner might be just before wakeup.
>>>>>>>>>>>>>>>>>>>>>>>>>> So I think it is more safe if 
>>>>>>>>>>>>>>>>>>>>>>>>>> GetObjectMonitorUsage() is called at 
>>>>>>>>>>>>>>>>>>>>>>>>>> safepoint in any case.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> Except we're moving away from safepoints to 
>>>>>>>>>>>>>>>>>>>>>>>>> using Handshakes, so this particular operation 
>>>>>>>>>>>>>>>>>>>>>>>>> will require that the apparent owner is 
>>>>>>>>>>>>>>>>>>>>>>>>> Handshake-safe (by entering a handshake with 
>>>>>>>>>>>>>>>>>>>>>>>>> it) before querying the monitor. This would 
>>>>>>>>>>>>>>>>>>>>>>>>> still be preferable I think to always using a 
>>>>>>>>>>>>>>>>>>>>>>>>> safepoint for the entire operation.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>>>>>>>>>>>> -----
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> [3] 
>>>>>>>>>>>>>>>>>>>>>>>>>> http://hg.openjdk.java.net/jdk/jdk/file/76a17c8143d8/src/hotspot/share/runtime/thread.cpp#l671
>>>>>>>>>>>>>>>>>>>>>>>>>> [4] 
>>>>>>>>>>>>>>>>>>>>>>>>>> http://hg.openjdk.java.net/jdk/jdk/file/76a17c8143d8/src/hotspot/share/runtime/thread.cpp#l684
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> However there is still a potential bug as 
>>>>>>>>>>>>>>>>>>>>>>>>>>> the thread reported as the owner may not be 
>>>>>>>>>>>>>>>>>>>>>>>>>>> suspended at the time we first see it, and 
>>>>>>>>>>>>>>>>>>>>>>>>>>> may release the monitor, but then it may get 
>>>>>>>>>>>>>>>>>>>>>>>>>>> suspended before we call:
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>   owning_thread = 
>>>>>>>>>>>>>>>>>>>>>>>>>>> Threads::owning_thread_from_monitor_owner(tlh.list(), 
>>>>>>>>>>>>>>>>>>>>>>>>>>> owner);
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> and so we think it is still the monitor 
>>>>>>>>>>>>>>>>>>>>>>>>>>> owner and proceed to query the monitor 
>>>>>>>>>>>>>>>>>>>>>>>>>>> information in a racy way. This can't happen 
>>>>>>>>>>>>>>>>>>>>>>>>>>> when suspension itself requires a safepoint 
>>>>>>>>>>>>>>>>>>>>>>>>>>> as the current thread won't go to that 
>>>>>>>>>>>>>>>>>>>>>>>>>>> safepoint during this code. However, if 
>>>>>>>>>>>>>>>>>>>>>>>>>>> suspension is implemented via a direct 
>>>>>>>>>>>>>>>>>>>>>>>>>>> handshake with the target thread then we 
>>>>>>>>>>>>>>>>>>>>>>>>>>> have a problem.
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>>>>>>>>>>>>>> -----
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> [1] 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> http://hg.openjdk.java.net/jdk/jdk/file/76a17c8143d8/src/hotspot/share/prims/jvmtiEnvBase.cpp#l973
>>>>>>>>>>>>>>>>>>>>>>>>>>>> [2] 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> http://hg.openjdk.java.net/jdk/jdk/file/76a17c8143d8/src/hotspot/share/prims/jvmtiEnvBase.cpp#l996
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>
>>>>
>>



More information about the serviceability-dev mailing list