RFR (S) 8209889: RedefineStress tests crash

[coleen.phillimore at oracle.com]

On 10/4/18 3:52 PM, dean.long at oracle.com wrote:
> On 10/4/18 11:28 AM, coleen.phillimore at oracle.com wrote:
>>
>>
>> On 10/4/18 2:00 PM, dean.long at oracle.com wrote:
>>> So having a raw CompileTask* is like having a raw Method*.  I'm 
>>> tempted to suggest a new CompiledTaskHandle, but it would only be 
>>> used in one place, so how about changing CompileTaskWrapper so we 
>>> could do something like this instead:
>>>
>>>     CompileTaskWrapper ctw(queue->get());
>>>     CompileTask* task = ctw.task();
>>
>> I started down this path but since CompileTaskWrapper cannot 
>> completely encapsulate CompileTask to make it safe, the code I have I 
>> think is just as good.  Also, adding NULL checks seems like it would 
>> allow some new behavoir that is now an invariant (ie CompileTask is 
>> never NULL).
>>
>
> OK.  Reviewed.

Thanks, Dean.
Coleen
>
> dl
>
>> Thanks,
>> Coleen
>>>
>>> CompileTaskWrapper would need to be able to handle a NULL task 
>>> gracefully.
>>>
>>> dl
>>>
>>> On 10/4/18 4:45 AM, coleen.phillimore at oracle.com wrote:
>>>> Summary: Create CompileTaskWrapper before potential safepoint
>>>>
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8209889.01/webrev
>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8209889
>>>>
>>>> MetadataOnStackMark needs to find the CompileTask in the current 
>>>> thread, or else the Method isn't marked as being alive, so if it's 
>>>> old/obsolete, it is freed.  There are checks to disable adding old 
>>>> methods to the compile queue but if the method is redefined after 
>>>> this point, there may be a preexisting bug that we compile an old 
>>>> method.  This change just fixes the crash.
>>>>
>>>> Can some compiler person (Martin?) look at where I moved 
>>>> possibly_add_compiler_threads and let me know if that looks okay? 
>>>> There's a comment in the change that says why you need to create 
>>>> the CompileTaskWrapper before that call.  I couldn't find a place 
>>>> to put a NoSafepointVerifier.
>>>>
>>>> Tested with the test that failed, which doesn't reproduce the bug 
>>>> except rarely, and test/hotspot/jtreg/compiler tests. tier1 an 2 in 
>>>> progress.
>>>>
>>>> Thanks,
>>>> Coleen
>>>
>>
>