Re: [PATCH] nptl: Fix racy pipe closing in tst-cancel{20,21}

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 02-12-2015 17:00, Florian Weimer wrote:
> On 10/14/2015 09:03 PM, Adhemerval Zanella wrote:
>> +  /* The pipe closing must be issued after the cancellation handling to avoid
>> +     a race condition where the cancellation runs after both pipe ends are
>> +     closed.  In this case the read syscall returns EOF and the cancellation
>> +     must not act.  */
> 
> I find this comment confusing.  What is actually happening during a
> spurious failure?  The thread which is supposed to be canceled sees a
> failed read from the pipe instead?

For current GLIBC cancellation mechanism it does not matter if the read
have side-effects or not, the cancellation will act regardless.  However
if we change the cancellation to *not* work if side-effects are returned 
from the syscall (which is the case for new cancellation mechanism I am 
currently working and the musl libc), then we must not cancel if the 
'read' syscall returns EOF.

This is the case of this testcase, where there is no synchronization
between the read in the thread and close in the main thread.  If the
cancellation signal handler acts *after* close syscalls, the read
on the other side of the pipe will return with side-effects (EOF). This
does not fail for GLIBC, even with the racy condition.
 

> 
> In this case, the return value from the read should be checked in a more
> stringent fashion.  Or is the read not supposed to return at all?  Then
> why have two read calls instead of one?

My understanding of the testcase if read to act as a blocking point
and to check if it is cancelled correctly.  As I put before, if the
syscall returns of not with side-effects, it does not matter to *GLIBC*.
However, since we are moving to a more robust cancellation mechanism
we must not cancel in this case.

Thus why I moved the closed after the thread is finished, if the
cancellation is not triggered, the testcase will still fail with a
timeout.

> 
> Florian
>