[9] RFR(XS): 8042570: Excessive number of tests timing out on nightly testing due to fix for 8040798

[Coleen Phillimore]

On 5/8/14, 1:12 AM, Albert wrote:
> On 05/08/2014 05:05 AM, David Holmes wrote:
>> On 7/05/2014 9:34 PM, Albert wrote:
>>> Hi,
>>>
>>> could I get reviews for this patch?
>>>
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8042570
>>>
>>> Problem:
>>> The fix of JDK-8040798 introduced a lock (MethodCompileQueue_lock) 
>>> prior
>>> to accessing the compilation queues
>>> of C1 and C2 in CompileBroker::mark_on_stack().
>>> CompileBroker::mark_on_stack() is executed at a safepoint.
>>> The locks that I introduced can cause the deadlock, since the
>>> MethodCompileQueue_lock lock is held at a safepoint
>>> that is reached from ciEnv::register_method(). This causes the 
>>> deadlock.
>>
>> This begs the question why you thought locking was necessary then and 
>> you no longer think it necessary now?
>>
> I introduced the lock in the first place, since compilation queues are 
> accessed by multiple threads and CompileQueue::mark_on_stack()
> modifies elements of the queue without acquiring a lock. However, I 
> did not realize that CompileQueue::mark_on_stack() is executed at
> a safepoint.

You might have to make sure that the compiler thread (that doesn't stop 
at safepoint if it doesn't have to), doesn't access these queues outside 
the safepoint though.   I don't think it will because the compiler 
threads transition into VM to touch Method* and other metadata.   I think.

Coleen
>
> Best,
> Albert
>> David
>> -----
>>
>>> I verified that deadlock happens as described above using the
>>> Redefine_v52_strict_invoke_redefine test.
>>>
>>> Solution:
>>> Remove locks in CompileBroker::mark_on_stack().
>>>
>>> Testing:
>>> Failing test cases.
>>>
>>> webrev:
>>> http://cr.openjdk.java.net/~anoll/8042570/webrev.00/
>>>
>>> Many thanks in advance and sorry having introduced this bug,
>>> Albert
>