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

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

On 09-10-2015 15:47, Roland McGrath wrote:
> 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.

You are correct regarding tst-cancel2 changes and the test is correct as is.
The change was due a wrong errno/error setting that was correct when
reviewing the ARM port (which leaded to the 'Add NPTL cases for cancellation
failures cases'). New cancellation works correct with current testcase as
well.

For tst-cancel4 the pthread_testcase calls are required due partial write/send
on the sockets.

Now for tst-cancel20 and tst-cancel21 what is happening is the read call in
sh_body (the signal handler set by the test) is returning 0 with errno set
to (0) since __pthread_get_ip return a value higher than the 'syscall'
instruction in the new syscall cancel wrapper (syscall_cancel.S).  I am not
sure why the kernel is setting the value of PC after the syscall instruction
and it is a spurious error, so some runs does not really shows it. I also
observer and musl also shows the same behavior for same test. Suggestions?

> 
> 
> Thanks,
> Roland
>