Re: [PATCH 4/5] nptl: Fix sem_wait and sem_timedwait cancellation

[Torvald Riegel <triegel at redhat dot com>]

On Mon, 2016-08-22 at 11:27 -0300, Adhemerval Zanella wrote:
> This patch fixes both sem_wait and sem_timedwait cancellation point for
> uncontended case.  In this scenario only atomics are involved and thus
> the futex cancellable call is not issue and a pending cancellation signal
> is not handled.

I have added a comment on the BZ explaining why I think this is NOTABUG.
Essentially, if we don't need to suspend or block, there's no suspension
or blocking we need to be able to cancel.  Adding a one-time check is
not reliable anyway because the cancellation request could be incoming
right after this check.

Also, if we don't want to make assumptions about timing, there is no way
to send a cancellation request reliably after sem_wait has reached it's
cancellation point because sem_wait doesn't communicate when it started
(or reached the cancellation point).

I don't see much value to allow a user to cancel something that would
never wait, because the user already is able to never call sem_wait at
all.  Furthermore, relying on cancellation to take effect when sem_wait
would never wait would break if the user code called a sem_trywait right
before the sem_wait, which arguably would be quite inconsistent
semantics.

Another point to consider is that even if glibc's implementation added
bounded(!) spinning (without checking for cancellation) before
eventually using futexes with cancellation enabled, we would still
fulfill the cancellation requirements because we can argue that the
cancellation point just happens a little later.

> GLIBC testcases do have tests for uncontended cases, test-cancel12
> and test-cancel14.c, however both are flawed by adding another
> cancellation point just after thread pthread_cleanup_pop:

I think both tests should just be removed because they check for
behavior we do not need to implement.  Sorry for not realizing this when
I updated the semaphore implementation.