Re: [PATCH v4 01/21] nptl: Do not close the pipe on tst-cancel{2,3}

[Zack Weinberg <zackw@panix.com>]

On Fri, Apr 3, 2020 at 4:32 PM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> This can cause a SIGPIPE before SIGCANCEL is processed, which makes
> write fail and the thread return an non expected result.

1) This is a sensible explanation for tst-cancel2.c, but tst-cancel3.c
does a read, not a write; the commit message should explain why this
is an appropriate change for both files.  Something like

# These tests cancel a thread that's supposed to be blocked while
writing or reading from a pipe.  If we close the other end of the
pipe, the closure might be reported to the thread before the
cancellation, causing a spurious failure.

2) The intention of the closes, seems to have been to prevent these
test cases blocking forever in pthread_join if the cancel isn't
delivered.  We don't actually need to do that, because the 20-second
timeout built into the test harness will suffice to make the test fail
in that case, but it might be worth adding a comment somewhere,
explaining that we're relying on the timeout.

3) I think it's no longer necessary to ignore SIGPIPE after this
change, and I think it's no longer necessary to have a loop in 'tf',
please remove that code also.

4) There's still a race in these tests; the cancellation might or
might not yet be pending when the child thread calls read/write.
Abstractly, we should be testing both a cancellation that's already
pending when we reach the cancellation point, and a cancellation
that's delivered while the thread is blocked on I/O.  It's possible to
ensure that the cancellation is already pending with a mutex, e.g.

static pthread_mutex_t prep_gate = PTHREAD_MUTEX_INITIALIZER;
static int fd[2];

static void *
tf (void *arg)
{
  /* The buffer size must be larger than the pipe size so that the
     write blocks.  */
  char buf[100000];

  /* Once we acquire this mutex, a cancellation request will be
     pending for this thread.  (pthread_mutex_(un)lock are not
     cancellation points.)  */
  pthread_mutex_lock (&prep_gate);
  pthread_mutex_unlock (&prep_gate);

  /* This write operation should be immediately cancelled.  */
  write (fd[1], buf, sizeof (buf));

  /* If control reaches this point, the test has failed;
     the parent will detect this.  */
  return arg;
}

static int
do_test (void)
{
  pthread_t th;
  void *r;

  if (pipe (fd) != 0)
    {
      puts ("pipe failed");
      return 1;
    }

  /* The child thread will wait to acquire this mutex.  */
  pthread_mutex_lock (&prep_gate);

  if (pthread_create (&th, NULL, tf, NULL) != 0)
    {
      puts ("create failed");
      return 1;
    }

  if (pthread_cancel (th) != 0)
    {
      puts ("cancel failed");
      return 1;
    }

  pthread_mutex_unlock (&prep_gate);

  // ...
}

But I don't know a good way to _guarantee_ that 'tf' is blocked on
read/write before the parent calls pthread_cancel.   Can you think of
anything?

zw