RFR(s): 8204166: TLH: Semaphore may not be destroy until signal have returned.

[Robbin Ehn]

Thanks, again!

/Robbin

On 06/18/2018 03:12 PM, Erik Österlund wrote:
> Hi Robbin,
> 
> Looks good.
> 
> Thanks,
> /Erik
> 
> On 2018-06-18 15:07, Robbin Ehn wrote:
>> Hi all,
>>
>> After some internal discussions I changed the patch to:
>> http://rehn-ws.se.oracle.com/cr_mirror/8204166/v2/
>>
>> Which handles thread off javathreads list better.
>>
>> Passes handshake testing and ZGC testing seems okay.
>>
>> Thanks, Robbin
>>
>> On 06/14/2018 12:11 PM, Robbin Ehn wrote:
>>> Hi all, please review.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8204166
>>> Webrev: http://cr.openjdk.java.net/~rehn/8204166/v1/webrev/
>>>
>>> The root cause of this failure is a bug in the posix semaphores: 
>>> https://sourceware.org/bugzilla/show_bug.cgi?id=12674
>>>
>>> Thread a:
>>> sem_post(my_sem);
>>>
>>> Thread b:
>>> sem_wait(my_sem);
>>> sem_destroy(my_sem);
>>>
>>> Thread b is waiting on my_sem (count 0), Thread a posts (count 0->1).
>>> If Thread b start executing directly after the increment in post but before
>>> Thread a leaves the call to post and manage to destroy the semaphore. Thread a
>>> _can_ get EINVAL from sem_post! This is fixed in newer glibc(2.21).
>>>
>>> Note that mutexes have had same issue on some platforms:
>>> https://sourceware.org/bugzilla/show_bug.cgi?id=13690
>>> Fixed in 2.23.
>>>
>>> Since we only have one handshake operation running at anytime (safepoints and 
>>> handshakes are also mutual exclusive, both run on VM Thread) we can actually 
>>> always use the same semaphore. This patch changes the _done semaphore to be 
>>> static instead, thus avoiding the post<->destroy race.
>>>
>>> Patch also contains some small changes which remove of dead code, remove 
>>> unneeded state, handling of cases which we can't easily say will never happen 
>>> and some additional error checks.
>>>
>>> Handshakes test passes, but they don't trigger the original issue, so more 
>>> interesting is that this issue do not happen when running ZGC which utilize 
>>> handshakes with the static semaphore.
>>>
>>> Thanks, Robbin
>