(13) RFR (XS): 8226917: jvmti/scenarios/contention/TC04/tc04t001/TestDescription.java fails on jvmti->InterruptThread (JVMTI_ERROR_THREAD_NOT_ALIVE)

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

(13) RFR (XS): 8226917: jvmti/scenarios/contention/TC04/tc04t001/TestDescription.java fails on jvmti->InterruptThread (JVMTI_ERROR_THREAD_NOT_ALIVE)

serguei.spitsyn@oracle.com
Please, review a test bug fix for:
https://bugs.openjdk.java.net/browse/JDK-8226917

Webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8226917-mon-events-correction2.1/ 



Summary:
   One more fragment in the native agent was overlooked and is not
needed anymore
   after the fix of JDK-8223736 which implemented more reliable sync
approach.
   The error code JVMTI_ERROR_THREAD_NOT_ALIVE started to be seen because
   the sleep timeout changed to be shorter. Now, the thread holding the
monitor
   is able to notice the enterEventsCount was increased by the
MonitorContendedEnter
   event callback and finish before the callback attempts to interrupt
it with the
   JVMTI InterruptThread.


Testing:
   A mach5 submission is in progress.

Thanks,
Serguei
Reply | Threaded
Open this post in threaded view
|

Re: (13) RFR (XS): 8226917: jvmti/scenarios/contention/TC04/tc04t001/TestDescription.java fails on jvmti->InterruptThread (JVMTI_ERROR_THREAD_NOT_ALIVE)

gary.adams@oracle.com
Looks good to me.

On 6/28/19 2:09 PM, [hidden email] wrote:

> Please, review a test bug fix for:
> https://bugs.openjdk.java.net/browse/JDK-8226917
>
> Webrev:
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8226917-mon-events-correction2.1/ 
>
>
>
> Summary:
>   One more fragment in the native agent was overlooked and is not
> needed anymore
>   after the fix of JDK-8223736 which implemented more reliable sync
> approach.
>   The error code JVMTI_ERROR_THREAD_NOT_ALIVE started to be seen because
>   the sleep timeout changed to be shorter. Now, the thread holding the
> monitor
>   is able to notice the enterEventsCount was increased by the
> MonitorContendedEnter
>   event callback and finish before the callback attempts to interrupt
> it with the
>   JVMTI InterruptThread.
>
>
> Testing:
>   A mach5 submission is in progress.
>
> Thanks,
> Serguei

Reply | Threaded
Open this post in threaded view
|

Re: (13) RFR (XS): 8226917: jvmti/scenarios/contention/TC04/tc04t001/TestDescription.java fails on jvmti->InterruptThread (JVMTI_ERROR_THREAD_NOT_ALIVE)

Daniel D. Daugherty
In reply to this post by serguei.spitsyn@oracle.com
On 6/28/19 2:09 PM, [hidden email] wrote:
> Please, review a test bug fix for:
> https://bugs.openjdk.java.net/browse/JDK-8226917
>
> Webrev:
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8226917-mon-events-correction2.1/ 
>

test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/contention/TC04/tc04t001/tc04t001.cpp
     old L129:         if
(!NSK_JVMTI_VERIFY(jvmti->GetObjectMonitorUsage(obj, &usageInfo))) {
     old L131:         } else if (usageInfo.owner != NULL) {
     old L132:             if
(!NSK_JVMTI_VERIFY(jvmti->InterruptThread(usageInfo.owner)))
         This construct in the old code is racy anyway. Just because the
         GetObjectMonitorUsage() returns information that the object is
         locked by the owner field does not mean that the owner thread
         hasn't exited the lock (and left the building) by the time the
         the InterruptThread() call is made. I suspect they were trying
         to get the owner thread out of the sleep, but they never should
         have checked the result of that call.

         Looks good.

Thumbs up.

Dan


>
>
> Summary:
>   One more fragment in the native agent was overlooked and is not
> needed anymore
>   after the fix of JDK-8223736 which implemented more reliable sync
> approach.
>   The error code JVMTI_ERROR_THREAD_NOT_ALIVE started to be seen because
>   the sleep timeout changed to be shorter. Now, the thread holding the
> monitor
>   is able to notice the enterEventsCount was increased by the
> MonitorContendedEnter
>   event callback and finish before the callback attempts to interrupt
> it with the
>   JVMTI InterruptThread.
>
>
> Testing:
>   A mach5 submission is in progress.
>
> Thanks,
> Serguei

Reply | Threaded
Open this post in threaded view
|

Re: (13) RFR (XS): 8226917: jvmti/scenarios/contention/TC04/tc04t001/TestDescription.java fails on jvmti->InterruptThread (JVMTI_ERROR_THREAD_NOT_ALIVE)

serguei.spitsyn@oracle.com
In reply to this post by gary.adams@oracle.com
Thank you a lot, Gary!
Serguei

On 6/28/19 11:58 AM, [hidden email] wrote:

> Looks good to me.
>
> On 6/28/19 2:09 PM, [hidden email] wrote:
>> Please, review a test bug fix for:
>> https://bugs.openjdk.java.net/browse/JDK-8226917
>>
>> Webrev:
>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8226917-mon-events-correction2.1/ 
>>
>>
>>
>> Summary:
>>   One more fragment in the native agent was overlooked and is not
>> needed anymore
>>   after the fix of JDK-8223736 which implemented more reliable sync
>> approach.
>>   The error code JVMTI_ERROR_THREAD_NOT_ALIVE started to be seen because
>>   the sleep timeout changed to be shorter. Now, the thread holding
>> the monitor
>>   is able to notice the enterEventsCount was increased by the
>> MonitorContendedEnter
>>   event callback and finish before the callback attempts to interrupt
>> it with the
>>   JVMTI InterruptThread.
>>
>>
>> Testing:
>>   A mach5 submission is in progress.
>>
>> Thanks,
>> Serguei
>

Reply | Threaded
Open this post in threaded view
|

Re: (13) RFR (XS): 8226917: jvmti/scenarios/contention/TC04/tc04t001/TestDescription.java fails on jvmti->InterruptThread (JVMTI_ERROR_THREAD_NOT_ALIVE)

serguei.spitsyn@oracle.com
In reply to this post by Daniel D. Daugherty
On 6/28/19 12:33 PM, Daniel D. Daugherty wrote:

> On 6/28/19 2:09 PM, [hidden email] wrote:
>> Please, review a test bug fix for:
>> https://bugs.openjdk.java.net/browse/JDK-8226917
>>
>> Webrev:
>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8226917-mon-events-correction2.1/ 
>>
>
> test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/contention/TC04/tc04t001/tc04t001.cpp
>
>     old L129:         if
> (!NSK_JVMTI_VERIFY(jvmti->GetObjectMonitorUsage(obj, &usageInfo))) {
>     old L131:         } else if (usageInfo.owner != NULL) {
>     old L132:             if
> (!NSK_JVMTI_VERIFY(jvmti->InterruptThread(usageInfo.owner)))
>         This construct in the old code is racy anyway. Just because the
>         GetObjectMonitorUsage() returns information that the object is
>         locked by the owner field does not mean that the owner thread
>         hasn't exited the lock (and left the building) by the time the
>         the InterruptThread() call is made. I suspect they were trying
>         to get the owner thread out of the sleep, but they never should
>         have checked the result of that call.

Yes, they wanted to get the owner thread out of the sleep.
And it is racy. There was pretty big timeout before to make it more
reliable.

Thanks,
Serguei

>
>         Looks good.
>
> Thumbs up.
>

Thanks a lot, Dan!
Serguei

> Dan
>
>
>>
>>
>> Summary:
>>   One more fragment in the native agent was overlooked and is not
>> needed anymore
>>   after the fix of JDK-8223736 which implemented more reliable sync
>> approach.
>>   The error code JVMTI_ERROR_THREAD_NOT_ALIVE started to be seen because
>>   the sleep timeout changed to be shorter. Now, the thread holding
>> the monitor
>>   is able to notice the enterEventsCount was increased by the
>> MonitorContendedEnter
>>   event callback and finish before the callback attempts to interrupt
>> it with the
>>   JVMTI InterruptThread.
>>
>>
>> Testing:
>>   A mach5 submission is in progress.
>>
>> Thanks,
>> Serguei
>

Reply | Threaded
Open this post in threaded view
|

Re: (13) RFR (XS): 8226917: jvmti/scenarios/contention/TC04/tc04t001/TestDescription.java fails on jvmti->InterruptThread (JVMTI_ERROR_THREAD_NOT_ALIVE)

Alex Menkov-2
In reply to this post by gary.adams@oracle.com
+1

--alex

On 06/28/2019 11:58, [hidden email] wrote:

> Looks good to me.
>
> On 6/28/19 2:09 PM, [hidden email] wrote:
>> Please, review a test bug fix for:
>> https://bugs.openjdk.java.net/browse/JDK-8226917
>>
>> Webrev:
>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8226917-mon-events-correction2.1/ 
>>
>>
>>
>> Summary:
>>   One more fragment in the native agent was overlooked and is not
>> needed anymore
>>   after the fix of JDK-8223736 which implemented more reliable sync
>> approach.
>>   The error code JVMTI_ERROR_THREAD_NOT_ALIVE started to be seen because
>>   the sleep timeout changed to be shorter. Now, the thread holding the
>> monitor
>>   is able to notice the enterEventsCount was increased by the
>> MonitorContendedEnter
>>   event callback and finish before the callback attempts to interrupt
>> it with the
>>   JVMTI InterruptThread.
>>
>>
>> Testing:
>>   A mach5 submission is in progress.
>>
>> Thanks,
>> Serguei
>
Reply | Threaded
Open this post in threaded view
|

Re: (13) RFR (XS): 8226917: jvmti/scenarios/contention/TC04/tc04t001/TestDescription.java fails on jvmti->InterruptThread (JVMTI_ERROR_THREAD_NOT_ALIVE)

serguei.spitsyn@oracle.com
Thanks a lot, Alex!
Serguei

On 6/28/19 3:56 PM, Alex Menkov wrote:

> +1
>
> --alex
>
> On 06/28/2019 11:58, [hidden email] wrote:
>> Looks good to me.
>>
>> On 6/28/19 2:09 PM, [hidden email] wrote:
>>> Please, review a test bug fix for:
>>> https://bugs.openjdk.java.net/browse/JDK-8226917
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8226917-mon-events-correction2.1/ 
>>>
>>>
>>>
>>> Summary:
>>>   One more fragment in the native agent was overlooked and is not
>>> needed anymore
>>>   after the fix of JDK-8223736 which implemented more reliable sync
>>> approach.
>>>   The error code JVMTI_ERROR_THREAD_NOT_ALIVE started to be seen
>>> because
>>>   the sleep timeout changed to be shorter. Now, the thread holding
>>> the monitor
>>>   is able to notice the enterEventsCount was increased by the
>>> MonitorContendedEnter
>>>   event callback and finish before the callback attempts to
>>> interrupt it with the
>>>   JVMTI InterruptThread.
>>>
>>>
>>> Testing:
>>>   A mach5 submission is in progress.
>>>
>>> Thanks,
>>> Serguei
>>