From owner-svn-src-all@FreeBSD.ORG Tue Apr 16 21:15:07 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 95A12B01; Tue, 16 Apr 2013 21:15:07 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (relay02.stack.nl [IPv6:2001:610:1108:5010::104]) by mx1.freebsd.org (Postfix) with ESMTP id 1724217CB; Tue, 16 Apr 2013 21:15:07 +0000 (UTC) Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131]) by mx1.stack.nl (Postfix) with ESMTP id 2E0CC359319; Tue, 16 Apr 2013 23:15:04 +0200 (CEST) Received: by snail.stack.nl (Postfix, from userid 1677) id 150292848C; Tue, 16 Apr 2013 23:15:04 +0200 (CEST) Date: Tue, 16 Apr 2013 23:15:03 +0200 From: Jilles Tjoelker To: John Baldwin Subject: Re: svn commit: r249566 - in head: lib/libc/gen sys/kern Message-ID: <20130416211503.GA1025@stack.nl> References: <201304162026.r3GKQVgs093874@svn.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201304162026.r3GKQVgs093874@svn.freebsd.org> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Apr 2013 21:15:07 -0000 On Tue, Apr 16, 2013 at 08:26:31PM +0000, John Baldwin wrote: > Author: jhb > Date: Tue Apr 16 20:26:31 2013 > New Revision: 249566 > URL: http://svnweb.freebsd.org/changeset/base/249566 > Log: > - Document that sem_wait() can fail with EINTR if it is interrupted by a > signal. > - Fix the old ksem implementation for POSIX semaphores to not restart > sem_wait() or sem_timedwait() if interrupted by a signal. > MFC after: 1 week > Modified: > head/lib/libc/gen/sem_wait.3 > head/sys/kern/uipc_sem.c > Modified: head/lib/libc/gen/sem_wait.3 > ============================================================================== > --- head/lib/libc/gen/sem_wait.3 Tue Apr 16 20:21:02 2013 (r249565) > +++ head/lib/libc/gen/sem_wait.3 Tue Apr 16 20:26:31 2013 (r249566) > @@ -27,7 +27,7 @@ > .\" > .\" $FreeBSD$ > .\" > -.Dd February 15, 2000 > +.Dd April 16, 2013 > .Dt SEM_WAIT 3 > .Os > .Sh NAME > @@ -75,6 +75,14 @@ points to an invalid semaphore. > .El > .Pp > Additionally, > +.Fn sem_wait > +will fail if: > +.Bl -tag -width Er > +.Pp > +.It Bq Er EINTR > +A signal interrupted this function. > +.El > +Additionally, > .Fn sem_trywait > will fail if: > .Bl -tag -width Er > > Modified: head/sys/kern/uipc_sem.c > ============================================================================== > --- head/sys/kern/uipc_sem.c Tue Apr 16 20:21:02 2013 (r249565) > +++ head/sys/kern/uipc_sem.c Tue Apr 16 20:26:31 2013 (r249566) > @@ -846,6 +846,8 @@ kern_sem_wait(struct thread *td, semid_t > err: > mtx_unlock(&sem_lock); > fdrop(fp, td); > + if (error == ERESTART) > + error = EINTR; > DP(("<<< kern_sem_wait leaving, pid=%d, error = %d\n", > (int)td->td_proc->p_pid, error)); > return (error); It would normally be expected (and required by POSIX) that a signal with SA_RESTART set does not cause a function to fail with [EINTR], so more rationale is needed here. Also, the timeouts passed to these functions are absolute and also are at the system call level except UMTX_OP_SEM_WAIT which can take a relative or an absolute timeout but libc passed it an absolute timeout. So there is no need for a caller to see [EINTR] to avoid overlong timeouts. Given the number of times people call sem_wait() without checking the return value, call sem_wait() and assume any error is fatal or call sem_timedwait() and assume any error is a timeout, I think it may be better to go the other way and never fail with [EINTR], just like the pthread functions. POSIX explicitly permits this: the [EINTR] error for sem_wait(), sem_trywait() and sem_timedwait() is "may fail" rather than the usual "shall fail" (note that SA_RESTART overrides such a "shall fail" unless there is a relative timeout or it is explicitly excluded with the function). I realize that this reduces functionality, but relying on [EINTR] here introduces a race condition anyway (because the signal handler may be called just before sem_wait() blocks). I suggest pthread cancellation, not returning from the signal handler or leaving the default action in place (if it is termination). The racy functionality can be kept by leaving [EINTR] and [ERESTART] from the sleep function as they are except for changing [ERESTART] to [EINTR] when UMTX_OP_SEM_WAIT was passed a relative timeout. Breakage in programs that check sem_wait()'s return value incorrectly will then be less frequent and only present when POSIX permits it. -- Jilles Tjoelker