From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailbackend.panix.com (mailbackend.panix.com [166.84.1.89]) by sourceware.org (Postfix) with ESMTPS id 163353877034 for ; Tue, 7 Apr 2020 15:25:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 163353877034 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=panix.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=zackw@panix.com Received: from mail-ed1-f50.google.com (mail-ed1-f50.google.com [209.85.208.50]) by mailbackend.panix.com (Postfix) with ESMTPSA id 48xWS55CX0zq57 for ; Tue, 7 Apr 2020 11:25:09 -0400 (EDT) Received: by mail-ed1-f50.google.com with SMTP id cw6so4501483edb.9 for ; Tue, 07 Apr 2020 08:25:09 -0700 (PDT) X-Gm-Message-State: AGi0PuZouI8T6obljHh9EXSul9aT8R1wc/2UVTVrZr2RfGOd6XjKrMfb P5neuQMCGQkRy1kDiiaIW/oYn4NZ7p7Tfk3zc4M= X-Google-Smtp-Source: APiQypLn4hP/vOZmnTuDtBBqWSwX3PFRs+K4GypxkykNk9B/riusygscQMymIA3n+cdk00et5oA0UwTVc772kfvAquA= X-Received: by 2002:a05:6402:6d7:: with SMTP id n23mr2335691edy.21.1586273108787; Tue, 07 Apr 2020 08:25:08 -0700 (PDT) MIME-Version: 1.0 References: <20200403203201.7494-1-adhemerval.zanella@linaro.org> <20200403203201.7494-2-adhemerval.zanella@linaro.org> In-Reply-To: <20200403203201.7494-2-adhemerval.zanella@linaro.org> From: Zack Weinberg Date: Tue, 7 Apr 2020 11:24:57 -0400 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v4 01/21] nptl: Do not close the pipe on tst-cancel{2,3} To: Adhemerval Zanella Cc: GNU C Library Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-1.4 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Apr 2020 15:25:18 -0000 On Fri, Apr 3, 2020 at 4:32 PM Adhemerval Zanella via Libc-alpha 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