RFR (S): 8023461: Thread holding lock at safepoint that vm can block on: MethodCompileQueue_lock

[Igor Veresov]

I vaguely remember that is was allowed before. That’s basically the reason why everything has handles in the policy. I need to recall how that works...

Btw, I may be wrong but it seems like there could be a race in MethodCounters creation. There is a similar problem with MDO, but we take a lock for it to avoid races.

igor

On Mar 11, 2014, at 3:04 PM, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> wrote:

> The policy for a thread is not to hold any locks VM can block on when entering a safepoint (see Thread::check_for_valid_safepoint_state).
> 
> Otherwise we would need to be very careful about what code can be executed during a safepoint to avoid deadlocks.
> 
> There are exceptions (like Threads_lock and Compile_lock), but generally we try to adhere the rule.
> 
> Making an exception for MethodCompileQueue looks safe (I went through the code and didn't find any scenarios when VM can attempt to grab it during a safepoint), but I'd like to avoid it if possible.
> 
> Best regards,
> Vladimir Ivanov
> 
> On 3/11/14 10:50 PM, Igor Veresov wrote:
>> Could you please remind me why we can’t enter a safepoint while holding the MethodCompileQueue_lock?
>> 
>> igor
>> 
>> On Mar 11, 2014, at 8:50 AM, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> wrote:
>> 
>>> Unfortunately, it's not enough. There's another safepoint check.
>>> 
>>> For blocking compilation requests of stale methods CompileTaskWrapper (see AdvancedThresholdPolicy::select_task) sends a notification to blocked threads after cancelling the compilation. It can safepoint while locking on compile task before sending notification.
>>> 
>>> I don't see how to avoid this situation. Any ideas?
>>> Otherwise, I need to exclude MethodCompileQueue from the check in Thread::check_for_valid_safepoint_state.
>>> 
>>> Best regards,
>>> Vladimir Ivanov
>>> 
>>> On 3/11/14 11:58 AM, Vladimir Ivanov wrote:
>>>> Igor, Vladimir, thanks for review.
>>>> 
>>>> Best regards,
>>>> Vladimir Ivanov
>>>> 
>>>> On 3/11/14 7:31 AM, Igor Veresov wrote:
>>>>> I think it’s a reasonable fix.
>>>>> 
>>>>> igor
>>>>> 
>>>>> On Mar 10, 2014, at 4:57 PM, Vladimir Ivanov
>>>>> <vladimir.x.ivanov at oracle.com> wrote:
>>>>> 
>>>>>> Vladimir, thanks for the review.
>>>>>> 
>>>>>> You are absolutely right about
>>>>>> Method::increment_interpreter_invocation_count. Reverted the change.
>>>>>> 
>>>>>> Updated fix:
>>>>>> http://cr.openjdk.java.net/~vlivanov/8023461/webrev.01/
>>>>>> 
>>>>>> Yes, Igor's feedback on this change would be invaluable.
>>>>>> 
>>>>>> Best regards,
>>>>>> Vladimir Ivanov
>>>>>> 
>>>>>> On 3/11/14 2:33 AM, Vladimir Kozlov wrote:
>>>>>>> The method Method::increment_interpreter_invocation_count(TRAP) changes
>>>>>>> are incorrect. It is used by C++ Interpreter and you did not modified
>>>>>>> code there. I would leave this method unchanged.
>>>>>>> 
>>>>>>> The rest looks fine to me but Igor should know better this code.
>>>>>>> 
>>>>>>> Thanks,
>>>>>>> Vladimir K
>>>>>>> 
>>>>>>> On 3/7/14 8:26 AM, Vladimir Ivanov wrote:
>>>>>>>> http://cr.openjdk.java.net/~vlivanov/8023461/webrev.00
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8023461
>>>>>>>> 42 lines changed: 13 ins; 1 del; 28 mod
>>>>>>>> 
>>>>>>>> The rule of thumb for VM is that a thread shouldn't hold any VM lock
>>>>>>>> when it reaches a safepoint. It's not the case for
>>>>>>>> MethodCompileQueue_lock now.
>>>>>>>> 
>>>>>>>> The problem is that AdvancedThresholdPolicy updates task's rate when
>>>>>>>> iterating compiler queue. It holds MethodCompileQueue_lock while doing
>>>>>>>> so. Method counters are allocated lazily. If method counters aren't
>>>>>>>> there and VM fails to allocate them, GC is initiated (see
>>>>>>>> CollectorPolicy::satisfy_failed_metadata_allocation) and a thead
>>>>>>>> entering a safepoint holding MethodCompileQueue lock.
>>>>>>>> 
>>>>>>>> Normally, counters are initialized during method interpretation,
>>>>>>>> but in
>>>>>>>> Xcomp mode it's not the case. That's the mode where the failures are
>>>>>>>> observed.
>>>>>>>> 
>>>>>>>> The fix is to skip the update, if counters aren't allocated yet.
>>>>>>>> 
>>>>>>>> Testing: added No_Safepoint_Verifier, JPRT, failing tests from nightly
>>>>>>>> testing (in progress).
>>>>>>>> 
>>>>>>>> Best regards,
>>>>>>>> Vladimir Ivanov
>>>>> 
>>