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

[Nils Eliasson]

Yes, good catch.

Next: http://cr.openjdk.java.net/~neliasso/8058461/webrev.09/

//Nils

On 2014-09-18 12:21, Albert wrote:
> Hi Nils,
>
>   785 void CompileBroker::print_compile_queues(outputStream* st) {
>   786   _c1_compile_queue->print_with_lock(st);
>   787   _c2_compile_queue->print_with_lock(st);
>   788 }
>
>
> can't it happen that _c1_compile_queue and/or _c2_compile_queue or NULL?
>
> Best,
> Albert
>
>
> On 09/18/2014 10:50 AM, Nils Eliasson wrote:
>> 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
>>>>>
>>>>
>>
>

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