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

David Holmes david.holmes at oracle.com
Thu Feb 11 22:50:39 UTC 2016


On 12/02/2016 8:46 AM, David Holmes wrote:
> 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. :(

No I take that back. I'll need to investigate the test failures more 
closely on Monday.

Thanks again,
David

> 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