[10] RFR: 8186018: SA: Monitor Cache Dump in HSDB does not work

[David Holmes]

Hi,

On 9/08/2017 4:55 PM, Yasumasa Suenaga wrote:
> Hi David,
> 
> I uploaded new webrev:
> 
>    http://cr.openjdk.java.net/~ysuenaga/JDK-8186018/webrev.01/
> 
> 2017-08-09 15:32 GMT+09:00 David Holmes <david.holmes at oracle.com>:
>> Hi Yasumasa,
>>
>> On 9/08/2017 3:54 PM, Yasumasa Suenaga wrote:
>>>
>>> Hi all,
>>>
>>> I tried to check Monitor Cache Dump in HSDB, but it hangs and OOME
>>> occurred.
>>>
>>> How to reproduce:
>>>
>>>     1. Run JShell
>>>     2. Attach HSDB to JShell
>>>        $ jhsdb hsdb --pid <PID>
>>>     3. Select "Monitor Cache Dump" in "Tools" menu.
>>>
>>> ObjectSynchronizer$ObjectMonitorIterator switches monitor block list
>>> when index == 0 at next(). However next() switches blockAddr only.
>>> We should also switch "block".
>>>
>>> I uloaded webrev for this issue. Could you review it?
>>>
>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8186018/webrev.00/
>>
>>
>> The fix looks good. Can you please add a comment inserted at L112:
>>
>> // advance to next block
>>
>> Also update copyright year.
> 
> Fixed them.

Copyright format is just two years: first and last, so should be:

* Copyright (c) 2001, 2017, Oracle and/or its affiliates. All rights 
reserved.

> 
>> Not sure if we need to do anything explicit about calling next() when we've
>> reached the end of the last block? Current code will throw NPE at L116, new
>> code will either throw NPE at L116 or perhaps at L113 if ObjectMonitor
>> constructor doesn't take null.
> 
> According to JavaDoc [1], next() throws NoSuchElementException if the
> iterator has no more elements.
> So I modified to throw this exception if blockAddr is null.

That looks good to me.

Thanks,
David

> 
> Thanks,
> 
> Yasumasa
> 
> 
> [1] https://docs.oracle.com/javase/8/docs/api/java/util/Iterator.html#next--
> 
> 
>> Thanks,
>> David
>>
>>
>>> I cannot access JPRT. So I need a sponsor.
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>