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