RFR (XS) 8213383 : Wrap up pthread_cond_wait into a loop to workaround potential spurious wakeups

[David Holmes]

Hi Ivan,

On 9/11/2018 10:40 AM, Ivan Gerasimov wrote:
> Thanks Serguei!
> 
> 
> On 11/6/18 1:01 PM, serguei.spitsyn at oracle.com wrote:
>> Hi Ivan,
>>
>> The fix looks good to me.
>> It is also possible to rewrite it like this:
>>    while (jvm_signal_installing && tid != pthread_self()) {
>>      pthread_cond_wait(&cond, &mutex);
>>    }
>>
> That's true.
> I think that having the nested loop shows more clear for what exact 
> condition we're waiting in the cond_wait.
> 
>> We should not care about wasting on execution of the condition
>> tid != pthread_self() as spurious wakeups have to be rare.
>>
> Also I just noticed that the comparison tid != pthread_self() is not 
> quite accurate, according to the documentation.
> Here's the except from the man 3 pthread_equals():
> """
> The  pthread_equal()  function  is  necessary  because  thread IDs 
> should be considered opaque: there is no
> portable way for applications to directly compare two pthread_t values.
> """

Yes that's quite true. We're guilty of doing this in the VM too ... we 
treat pthread_t as a pointer type in places (for printing primarily)

> Please find the updated webrev:
> http://cr.openjdk.java.net/~igerasim/8213383/01/webrev/

Looks good to me.

Thanks,
David

> With kind regards,
> Ivan
> 
>> Thanks,
>> Serguei
>>
>>
>> On 11/5/18 5:20 PM, Ivan Gerasimov wrote:
>>> Thank you Dean!
>>>
>>>
>>> On 11/5/18 4:09 PM, dean.long at oracle.com wrote:
>>>> Looks good, but do you want to use a while loop instead of if-do-while?
>>>>
>>>>      if (tid != pthread_self()) {
>>>>        while (jvm_signal_installing) {
>>>>          pthread_cond_wait(&cond, &mutex);
>>>>        }
>>>>      }
>>>>
>>> This code will be executed before tid is initialized (see 
>>> JVM_begin_signal_setting()), so jvm_signal_installing has to be 
>>> tested first.
>>>
>>> With kind regards,
>>> Ivan
>>>
>>>> dl
>>>>
>>>> On 11/5/18 11:59 AM, Ivan Gerasimov wrote:
>>>>> Hello!
>>>>>
>>>>> In src/java.base/unix/native/libjsig/jsig.c:
>>>>>
>>>>> signal_lock() makes all the thread that are not jvm wait while the 
>>>>> later is installing signal handlers.
>>>>> This is done via a call to pthread_cond_wait().
>>>>> Spurious wakeups from pthread_cond_wait() are allowed, so it needs 
>>>>> to be wrapped up into a loop.
>>>>>
>>>>> Would you please help review the trivial fix?
>>>>> Control build and testing of tier1,2,3 went fine on all supported 
>>>>> platforms.
>>>>>
>>>>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8213383
>>>>> WEBREV: http://cr.openjdk.java.net/~igerasim/8213383/00/webrev/
>>>>>
>>>>> Thanks!
>>>>>
>>>>
>>>
>>
>>
>