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

[Vladimir Ivanov]

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