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

[Vladimir Ivanov]

Igor,

> 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...
It's there for a long time, but I converted the check from VM warning to 
fatal error only recently.

AdvancedThresholdPolicy::select_task operates on raw Method*. As I can 
see in the sources, handles are used only in 
Method::build_method_counters. Lazy allocation of method counters wasn't 
there originally. It was added by 8010862.

> 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.
You are right. There's a window in Method::build_method_counters when 
counters can be allocated twice. We need to grab a lock / use CAS to 
avoid memory leak here.

Best regards,
Vladimir Ivanov

[1] https://bugs.openjdk.java.net/browse/JDK-8010862

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