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

[Vladimir Kozlov]

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
>>
>