Re: [PATCH v2] nptl: Fix testcases for new pthread cancellation mechanism

[Roland McGrath <roland at hack dot frob dot com>]

I don't really understand the rationale for the pthread_testcancel changes.
They need more thorough comments in the test code.

For example, AFAICT tst-cancel2 is entirely a test that a blocked partial
write is interrupted by cancellation.  AIUI it the new intent is that this
write should return a partial result without triggering cancellation.  But
the write is in a loop, and the next iteration should fail with EPIPE.  In
the EPIPE case, no side effect of the syscall has already taken place, so
IMHO it should in fact act as a normal cancellation point.  In that case,
the test is already correct(*) as it stands.

If we do come to a consensus about this to contrary that opinion, then the
test ought to have specific expectations about how write will behave.  So
it should check that the write failed in the expected fashion (EPIPE).

(*) This test is racy now and racy in your proposed version, because there
is no way to be sure that the new thread has gotten into the write before
the main thread does pthread_cancel.  So really this test should document
thoroughly what it's intended to test and then we should make sure it is
reliably testing that.

I didn't look at the details of the logic in the other tests.  tst-cancel2
is by far the simplest of them, and just for that we have substantial
subtlety and unresolved issues about the test.  Each of these tests needs
careful consideration.  This is as much about the utility of the test in
the status quo ante as about what your changes do, but it's important that
we be sure what we're testing, why, and how, before evaluating subtle
changes to tests like the ones you propose.  I think this review will be
easier to do if we take one test at a time rather than trying to tackle
several in one patch and review session.


Thanks,
Roland