(S) RFR: 8029453: java/util/concurrent/locks/ReentrantLock/TimeoutLockLoops.java failed by timeout

[David Holmes]

Hi Dmitry,

On 13/08/2015 5:44 AM, Dmitry Dmitriev wrote:
> Hello David,
>
> Changes looks good, but I'm not a reviewer.

Thanks - still need a Reviewer please!

> By the way, I see one enhancement that can be made in Parker::unpark()
> that can be implemeted if you wish in this bug or later(for example,
> when code for WorkAroundNPTLTimedWaitHang will be removed).
> Parker::unpark() have duplicated code(lines 5796-5797 and 5800-5801):
> pthread_mutex_unlock(_mutex);
> assert(status == 0, "invariant");
>
> I think it can be removed by combining two if's on lines 5778 and 5780
> into one:
>   if ((s < 1) && (_cur_index != -1))
>
> Also return value of pthread_mutex_unlock not assinged to 'status' in
> this case.

I'll flag that for when the workaround code is removed - thanks. I 
prefer to just fix the current bug in the most minimal way. The asserts 
should be assert_status as well.

David

> Thank you,
> Dmitry
>
> On 11.08.2015 9:40, David Holmes wrote:
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8029453
>>
>> Webrev: http://cr.openjdk.java.net/~dholmes/8029453/webrev/
>>
>> The code introduced in 6900441 contained a bug in the code path for
>> when WorkAroundNPTLTimedWaitHang was zero, and this was exposed by the
>> change in 8130728 which made the default setting of
>> WorkAroundNPTLTimedWaitHang zero.
>>
>> In PlatformParker on Linux _cur_index tracks which pthread_cond object
>> is currently in use by a waiting thread (one for relative-timed waits,
>> the other for absolute-timed waits) and is set to -1 when the thread
>> is not waiting. In the path now used by default we release the
>> pthread_mutex_t and then pthread_cond_signal the condition variable at
>> _cond[_cur_index]. But as soon as we release the mutex the waiting
>> thread can resume execution (it may have timed-out and so not need the
>> signal) and set _cur_index to -1. The signalling thread then signals
>> _cond[-1] which does not contain a pthread_cond_t object. This can
>> result in the pthread_cond_signal hanging, and potentially other
>> consequences.
>>
>> The fix is simple: save the correct index before unlocking the mutex.
>>
>> The test:
>> java/util/concurrent/locks/ReentrantLock/TimeoutLockLoops.java has
>> been marked as failing intermittently (8133231) due to this and I will
>> revert that as part of this fix, once that change reaches the hs-rt
>> forest.
>>
>> Thanks,
>> David
>