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

[Albert]

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/04779d8b/attachment.html>