From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/13503 Path: news.gmane.org!.POSTED!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: sem_wait and EINTR Date: Wed, 5 Dec 2018 17:03:37 -0500 Message-ID: <20181205220337.GY23599@brightrain.aerifal.cx> References: <20181205191605.72492698@orivej.orivej.org> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: blaine.gmane.org 1544047308 24150 195.159.176.226 (5 Dec 2018 22:01:48 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Wed, 5 Dec 2018 22:01:48 +0000 (UTC) User-Agent: Mutt/1.5.21 (2010-09-15) To: musl@lists.openwall.com Original-X-From: musl-return-13519-gllmg-musl=m.gmane.org@lists.openwall.com Wed Dec 05 23:01:44 2018 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1gUfEi-00064r-I6 for gllmg-musl@m.gmane.org; Wed, 05 Dec 2018 23:01:40 +0100 Original-Received: (qmail 17498 invoked by uid 550); 5 Dec 2018 22:03:49 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 17480 invoked from network); 5 Dec 2018 22:03:49 -0000 Content-Disposition: inline In-Reply-To: <20181205191605.72492698@orivej.orivej.org> Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:13503 Archived-At: On Wed, Dec 05, 2018 at 07:16:05PM +0000, Orivej Desh wrote: > Hi, > > musl differs from glibc in that it does not return from sem_wait() on EINTR. > This mail [1] explains that this is useful to safeguard the software that does > not check sem_wait() return code. However, since glibc does return EINTR, such > bugs in the open source software seem to be eventually noticed and fixed. > > The musl behaviour has a disadvantage in that it makes sem_wait() difficult to > interrupt (and delays the return from sem_timedwait() until the timeout), which > is relied upon in particular by multithreaded fuse for breaking out of the > main thread waiting loop [2]. IMHO the fuse implementation is sensible, since it > looks better than the alternatives I could imagine, and I'm inclined to patch > musl like this [3] to meet its expectations. > > Am I missing some implications? Would you reconsider returning from sem_wait() > on EINTR? Could you suggest a good fix for fuse that does not change musl? > > [1] https://www.openwall.com/lists/musl/2018/02/24/3 > [2] https://github.com/libfuse/libfuse/blob/fuse-3.3.0/lib/fuse_loop_mt.c#L332 > [3] https://github.com/orivej/musl/commit/c4c38aaab4fc55c23669f7b81386b615609cc3e1 > > diff --git a/src/thread/sem_timedwait.c b/src/thread/sem_timedwait.c > index 8132eb1b..58d3ebfe 100644 > --- a/src/thread/sem_timedwait.c > +++ b/src/thread/sem_timedwait.c > @@ -22,7 +22,7 @@ int sem_timedwait(sem_t *restrict sem, const struct timespec *restrict at) > pthread_cleanup_push(cleanup, (void *)(sem->__val+1)); > r = __timedwait_cp(sem->__val, -1, CLOCK_REALTIME, at, sem->__val[2]); > pthread_cleanup_pop(1); > - if (r && r != EINTR) { > + if (r) { > errno = r; > return -1; > } > diff --git a/src/thread/synccall.c b/src/thread/synccall.c > index cc66bd24..d9ab40cb 100644 > --- a/src/thread/synccall.c > +++ b/src/thread/synccall.c > @@ -37,10 +37,10 @@ static void handler(int sig) > if (a_cas(&target_tid, ch.tid, 0) == (ch.tid | 0x80000000)) > __syscall(SYS_futex, &target_tid, FUTEX_UNLOCK_PI|FUTEX_PRIVATE); > > - sem_wait(&ch.target_sem); > + while (sem_wait(&ch.target_sem) && errno != EINTR); > callback(context); > sem_post(&ch.caller_sem); > - sem_wait(&ch.target_sem); > + while (sem_wait(&ch.target_sem) && errno != EINTR); > > errno = old_errno; > } > @@ -153,7 +153,7 @@ void __synccall(void (*func)(void *), void *ctx) > /* Serialize execution of callback in caught threads. */ > for (cp=head; cp; cp=cp->next) { > sem_post(&cp->target_sem); > - sem_wait(&cp->caller_sem); > + while (sem_wait(&cp->caller_sem) && errno != EINTR); > } > > sa.sa_handler = SIG_IGN; I think the changes to __synccall are unnecessary noise. It necessarily runs with all signals, even implementation-internal ones, blocked. Did you just miss this or do you think there's a reason the checks need to be added? Rich