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

[Dmitry Dmitriev]

Hello David,

Changes looks good, but I'm not a reviewer.

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.

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