summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRich Felker <dalias@aerifal.cx>2018-12-18 12:17:33 -0500
committerRich Felker <dalias@aerifal.cx>2018-12-18 12:17:33 -0500
commita63c0104e496f7ba78b64be3cd299b41e8cd427f (patch)
treed2d1952cb710fff180e2c98f33d61d1f04fcb461
parentc00cdefa1da17f60b3179704528582ef320e61b8 (diff)
downloadmusl-a63c0104e496f7ba78b64be3cd299b41e8cd427f.tar.gz
add __timedwait backend workaround for old kernels where futex EINTRs
prior to linux 2.6.22, futex wait could fail with EINTR even for non-interrupting (SA_RESTART) signals. this was no problem provided the caller simply restarted the wait, but sem_[timed]wait is required by POSIX to return when interrupted by a signal. commit a113434cd68ce30642c4995b1caadcd084be6f09 introduced this behavior, and commit c0ed5a201b2bdb6d1896064bec0020c9973db0a1 reverted it based on a mistaken belief that it was not required. this belief stems from a bug in the specification: the description requires the function to return when interrupted, but the errors section marks EINTR as a "may fail" condition rather than a "shall fail" one. since there does seem to be significant value in the change made in commit c0ed5a201b2bdb6d1896064bec0020c9973db0a1, making it so that programs that call sem_wait without checking for EINTR don't silently make forward progress without obtaining the semaphore or treat it as a fatal error and abort, add a behind-the-scenes mechanism in the __timedwait backend to suppress EINTR in programs that have never installed interrupting signal handlers, and have sigaction track and report this state. this way the semaphore code is not cluttered by workarounds and can be updated (to be done in next commit) to reflect the high-level logic for conforming behavior. these changes are based loosely on a patch by Markus Wichmann, with the main changes being atomic update to flag object and moving the workaround from sem_timedwait to the __timedwait futex backend.
-rw-r--r--src/internal/pthread_impl.h1
-rw-r--r--src/signal/sigaction.c6
-rw-r--r--src/thread/__timedwait.c8
3 files changed, 15 insertions, 0 deletions
diff --git a/src/internal/pthread_impl.h b/src/internal/pthread_impl.h
index 7a25b88e..58ecce90 100644
--- a/src/internal/pthread_impl.h
+++ b/src/internal/pthread_impl.h
@@ -156,6 +156,7 @@ extern hidden volatile int __block_new_threads;
extern hidden volatile size_t __pthread_tsd_size;
extern hidden void *__pthread_tsd_main[];
extern hidden volatile int __aio_fut;
+extern hidden volatile int __eintr_valid_flag;
hidden int __clone(int (*)(void *), void *, int, void *, ...);
hidden int __set_thread_area(void *);
diff --git a/src/signal/sigaction.c b/src/signal/sigaction.c
index af47195e..05445089 100644
--- a/src/signal/sigaction.c
+++ b/src/signal/sigaction.c
@@ -21,6 +21,8 @@ void __get_handler_set(sigset_t *set)
memcpy(set, handler_set, sizeof handler_set);
}
+volatile int __eintr_valid_flag;
+
int __libc_sigaction(int sig, const struct sigaction *restrict sa, struct sigaction *restrict old)
{
struct k_sigaction ksa, ksa_old;
@@ -43,6 +45,10 @@ int __libc_sigaction(int sig, const struct sigaction *restrict sa, struct sigact
SIGPT_SET, 0, _NSIG/8);
unmask_done = 1;
}
+
+ if (!(sa->sa_flags & SA_RESTART)) {
+ a_store(&__eintr_valid_flag, 1);
+ }
}
/* Changing the disposition of SIGABRT to anything but
* SIG_DFL requires a lock, so that it cannot be changed
diff --git a/src/thread/__timedwait.c b/src/thread/__timedwait.c
index 229db313..ae19bd63 100644
--- a/src/thread/__timedwait.c
+++ b/src/thread/__timedwait.c
@@ -5,6 +5,9 @@
#include "syscall.h"
#include "pthread_impl.h"
+static volatile int dummy = 0;
+weak_alias(dummy, __eintr_valid_flag);
+
int __timedwait_cp(volatile int *addr, int val,
clockid_t clk, const struct timespec *at, int priv)
{
@@ -28,6 +31,11 @@ int __timedwait_cp(volatile int *addr, int val,
r = -__syscall_cp(SYS_futex, addr, FUTEX_WAIT|priv, val, top);
if (r == ENOSYS) r = -__syscall_cp(SYS_futex, addr, FUTEX_WAIT, val, top);
if (r != EINTR && r != ETIMEDOUT && r != ECANCELED) r = 0;
+ /* Mitigate bug in old kernels wrongly reporting EINTR for non-
+ * interrupting (SA_RESTART) signal handlers. This is only practical
+ * when NO interrupting signal handlers have been installed, and
+ * works by sigaction tracking whether that's the case. */
+ if (r == EINTR && !__eintr_valid_flag) r = 0;
return r;
}