URGENT (XS) RFR: 8149697: Fix for 8145725 is broken

David Holmes david.holmes at oracle.com
Thu Feb 11 22:46:09 UTC 2016


Thanks for all the quick reviews everyone, and thanks Coleen for 
sponsoring this.

I had initially thought this could only lead to spurious wakeups, but I 
see now that it can lead to lost signals and so hangs. :(

Thanks,
David

On 12/02/2016 6:46 AM, Coleen Phillimore wrote:
> Thanks, Dan and Jerry.  I'll sponsor this for David who should be on
> vacation today.
> Coleen
>
> On 2/11/16 3:45 PM, Daniel D. Daugherty wrote:
>> Thumbs up!
>>
>> Dan
>>
>>
>> On 2/11/16 1:37 PM, David Holmes wrote:
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8149697
>>>
>>> webrev: http://cr.openjdk.java.net/~dholmes/8149697/webrev/
>>>
>>> In the refactoring for 8145725 a check of _cur_index was moved
>>> outside the mutex block, but this was wrong as the value could be
>>> changed by a parking thread. As per the existing comment we needed to
>>> check the saved 'index' instead:
>>>
>>> --- old/src/os/linux/vm/os_linux.cpp    2016-02-11 15:23:59.696802390
>>> -0500
>>> +++ new/src/os/linux/vm/os_linux.cpp    2016-02-11 15:23:57.440675175
>>> -0500
>>> @@ -5756,7 +5756,7 @@
>>>    int index = _cur_index;
>>>    status = pthread_mutex_unlock(_mutex);
>>>    assert_status(status == 0, status, "invariant");
>>> -  if (s < 1 && _cur_index != -1) {
>>> +  if (s < 1 && index != -1) {
>>>      // thread is definitely parked
>>>      status = pthread_cond_signal(&_cond[index]);
>>>      assert_status(status == 0, status, "invariant");
>>>
>>> This could introduce spurious wakeups for a parking thread.
>>>
>>> Thanks,
>>> David
>>
>


More information about the hotspot-runtime-dev mailing list