RFR(S): 8058461: serviceability/dcmd/CodelistTest.java and serviceability/dcmd/CompilerQueueTest.java SIGSEGV

[Nils Eliasson]

Yes. Also good to have the print method lock-free so it can be callled 
from a debugger without hassle. Adding a wrapper that takes the 
compileQueue lock.

New webrev: http://cr.openjdk.java.net/~neliasso/8058461/webrev.08/

Thanks!
//Nils

On 2014-09-18 02:59, Vladimir Kozlov wrote:
> CompileQueue::add() path already has lock acquired.
> I think you need the lock only in CompileBroker::print_compile_queues().
>
> Thanks,
> Vladimir
>
> On 9/17/14 1:54 AM, Nils Eliasson wrote:
>>
>> Hi Albert,
>>
>> On 2014-09-16 10:45, Albert wrote:
>>> Hi Nils,
>>>
>>> please see comments inline.
>>>
>>> On 09/16/2014 10:16 AM, Nils Eliasson wrote:
>>>> Hi,
>>>>
>>>> I would like review of this change that includes three fixes:
>>>>
>>>> 1) Let Dcmd Compiler.codelist only print alive-nmethods. We ran into
>>>> crashes when listing zombies and unloaded too. Alive nmethods
>>>> includes not-entrants so it still gives a pretty good idea about
>>>> whats in the code cache and what has been used recently.
>>>>
>>> What was the cause of these crashes? Is the reason that
>>> nmethod->method() is null?
>>
>> In one case it crashed when nmethod->method()->constants() was null when
>> printing the name.
>>
>> I see that the segmented code cache push already changed to only
>> iterating the alive nmethods so I removed that from the patch.
>>
>>> Would it make sense to print information about zombie methods that
>>> don't lead
>>> to a crash?
>> Probably not. Not_entrant nmethods may stay around for a while, but once
>> zombies they are reclaimed pretty fast.
>>
>>>
>>>
>>>> 2) Take CompileQueue lock when printing queue. It is not enough to be
>>>> at a safepoint - the compiler threads may still mutate the list
>>>> causing crashes.
>>>>
>>> Are we guaranteed to not run into a deadlock when we acquire the lock?
>>
>> Yes. It doesn't use any other lock.
>>
>> New webrev: http://cr.openjdk.java.net/~neliasso/8058461/webrev.05/
>>
>> Thanks Albert and Christian for your feedback.
>>
>> //Nils
>>
>>>
>>> Best,
>>> Albert
>>>> 3) Relax the parsing of long hex-numbers in the test of codelist.
>>>> High addresses (sparc) casues NumberFormatExceptions.
>>>>
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8058461
>>>> webrev: http://cr.openjdk.java.net/~neliasso/8058461/webrev.04/
>>>>
>>>> Thanks!
>>>> //Nils
>>>
>>