RFR(S): 8063112: Compiler diagnostic commands should have locking instead of safepoint

[Albert Noll]

Hi Nils,

thanks for your response.
On 11/10/2014 11:38 AM, Nils Eliasson wrote:
> Hi Albert,
>
> On 2014-11-10 11:10, Albert Noll wrote:
>> Hi Nils,
>>
>>
>> On 11/06/2014 02:22 PM, Nils Eliasson wrote:
>>> Hi,
>>>
>>> Please review this small change.
>>>
>>> The first implementation of compiler diagnostic commands was 
>>> asserting on safepoint or lock. The safepoint was always taken since 
>>> the dcmd by default block for a safepoint. That is not correct, the 
>>> lock must be taken, and a whether we are at a safepoint doesn't matter.
>>>
>> Could you please explain why we always need to take the lock?
>
> Mutation of the code cache can happen at any time, for example when a 
> new compiled method is installed. The codelist dcmd iterates the code 
> cache and might see a dangling pointer. The code cache stats dcmd only 
> print statisticss, so it won't break but can see bad data.
>
Here are two things that I don't understand:
1) assert_locked_or_safepoint(CodeCache_lock) checks if we are at a 
safepoint and/or we own the 'CodeCache_lock'. If we are at a safepoint, 
how can another thread possibly modify data structures in the code cache?

2) Why is it safe to use assert_locked_or_safepoint(CodeCache_lock) in 
so many places except for CodeCache::print_codelist(outputStream* st)?

3) To make sure that no other thread modifies the code cache, wouldn't 
you you have to make sure that every modification to the code cache is 
guarded by assert(CodeCache_lock->owned_by_self(), "Checking") instead 
of assert_locked_or_safepoint(CodeCache_lock)? I.e., a lock is only 
useful if every modification to the code cache is guarded by that lock.

Best,
Albert

> Regards,
> Nils
>
>>
>> Thanks,
>> Albert
>>> Tested with jtreg tests in Hotspot and JDK that exercise this code.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8063112
>>> Webrev: http://cr.openjdk.java.net/~neliasso/8063112/webrev.01/
>>>
>>> Thanks,
>>> Nils
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20141110/db7e1d54/attachment.html>