[10] RFR: 8185164: GetOwnedMonitorInfo() returns incorrect owned monitor

[David Holmes]

Hi Dan,

On 26/07/2017 11:52 PM, Daniel D. Daugherty wrote:
> On 7/26/17 12:11 AM, David Holmes wrote:
>> On 26/07/2017 10:27 AM, Yasumasa Suenaga wrote:
>>> Hi Dan,
>>>
>>>> I've added some analysis to the bug report
>>>
>>> Thanks!
>>> I tried to fix this issue in JvmtiEnvBase::get_owned_monitors() at 
>>> first.
>>> But it is difficult because we cannot know pending monitor if thread 
>>> state is MONITOR_CONTENDED_ENTER when get_owned_monitor() is called.
>>
>> I need to look closer at this when I get back from vacation next week.
> 
> Seems like you're back already. :-)
> 
> 
>> A pending monitor should not be reported as owned (unless the spec 
>> says otherwise) and it seems odd to me to fix the current problem by 
>> marking the monitor as pending earlier.
> 
> It's the updating of the _current_pending_monitor field that
> allows JvmtiEnvBase::get_locked_objects_in_frame() to recognize
> that the monitor observed in the frame is only pending and
> is not owned.
> 
> I put a fairly detailed note in the bug yesterday, but you
> should look at that when you're officially back!

Thanks for clarifying things. I also added a comment to the bug report.

I think the fix is sound and prevents anyone from observing the case 
where the monitor will be seen in the stack-frame, but has not yet been 
set as the "pending monitor". As far as I can tell it is only this case 
(GetOwnedMonitorInfo from the contended-monitor event callback in the 
current thread) that will be able to observe the change.

Thanks,
David

> Dan
> 
> 
>>
>> Thanks,
>> David
>>
>>>> Did you run the jdk repo's com/sun/jdi tests with your fix?
>>>
>>> I have not done yet.
>>> I have a trip until 28 July JST. So I will run it after that.
>>>
>>>
>>> Yasumasa
>>>
>>>
>>> On 2017/07/26 7:05, Daniel D. Daugherty wrote:
>>>> On 7/24/17 8:40 PM, Yasumasa Suenaga wrote:
>>>>> Hi all,
>>>>>
>>>>> I tried to get owned monitors in MonitorContendedEnter JVMTI event 
>>>>> handler.
>>>>> However GetOwnedMonitorInfo JVMTI function returns a monitor which is
>>>>> not yet owned.
>>>>>
>>>>> I attached reproducer to JBS. Please read README.md.
>>>>>
>>>>> I think GetOwnedMonitorInfo() should not return a pending monitor.
>>>>>
>>>>>
>>>>> I uploaded webrev. Could you review?
>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8185164/webrev.00/
>>>>>
>>>>> I hope this fix is applied to 8u or later release.
>>>>> I cannot access JPRT. So I need a sponsor.
>>>>
>>>> Thanks for the bug report. It's nice to have a test case and a proposed
>>>> fix all in the bug report! I've added some analysis to the bug report
>>>> and we'll need to run this fix through Oracle's JPDA test stack which
>>>> is not (yet) open.
>>>>
>>>> Did you run the jdk repo's com/sun/jdi tests with your fix?
>>>>
>>>> Dan
>>>>
>>>>
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>
>