optimize posix_spawn to avoid spurious sigaction syscalls

the trick here is that sigaction can track for us which signals have
ever had a signal handler set for them, and only those signals need to
be considered for reset. this tracking mask may have false positives,
since it is impossible to remove bits from it without race conditions.
false negatives are not possible since the mask is updated with atomic
operations prior to making the sigaction syscall.

implementation-internal signals are set to SIG_IGN rather than SIG_DFL
so that a signal raised in the parent (e.g. calling pthread_cancel on
the thread executing pthread_spawn) does not have any chance make it
to the child, where it would cause spurious termination by signal.

this change reduces the minimum/typical number of syscalls in the
child from around 70 to 4 (including execve). this should greatly
improve the performance of posix_spawn and other interfaces which use
it (popen and system).

to facilitate these changes, sigismember is also changed to return 0
rather than -1 for invalid signals, and to return the actual status of
implementation-internal signals. POSIX allows but does not require an
error on invalid signal numbers, and in fact returning an error tends
to confuse applications which wrongly assume the return value of
sigismember is boolean.

diff --git a/src/process/posix_spawn.c b/src/process/posix_spawn.c
index c688652672fe507ca93d4de29d5494b234497cfc..eb98f9f60725d077852fbd55bc24b5e57493f0e4 100644 (file)
--- a/src/process/posix_spawn.c
+++ b/src/process/posix_spawn.c
@@ -20,29 +20,43 @@ struct args {
        char *const *argv, *const *envp;
 };
 
+void __get_handler_set(sigset_t *);
+
 static int child(void *args_vp)
 {
        int i, ret;
-       struct sigaction sa;
+       struct sigaction sa = {0};
        struct args *args = args_vp;
        int p = args->p[1];
        const posix_spawn_file_actions_t *fa = args->fa;
        const posix_spawnattr_t *restrict attr = args->attr;
+       sigset_t hset;
 
        close(args->p[0]);
 
        /* All signal dispositions must be either SIG_DFL or SIG_IGN
         * before signals are unblocked. Otherwise a signal handler
         * from the parent might get run in the child while sharing
-        * memory, with unpredictable and dangerous results. */
+        * memory, with unpredictable and dangerous results. To
+        * reduce overhead, sigaction has tracked for us which signals
+        * potentially have a signal handler. */
+       __get_handler_set(&hset);
        for (i=1; i<_NSIG; i++) {
-               __libc_sigaction(i, 0, &sa);
-               if (sa.sa_handler!=SIG_DFL && (sa.sa_handler!=SIG_IGN ||
-                   ((attr->__flags & POSIX_SPAWN_SETSIGDEF)
-                    && sigismember(&attr->__def, i) ))) {
+               if ((attr->__flags & POSIX_SPAWN_SETSIGDEF)
+                    && sigismember(&attr->__def, i)) {
                        sa.sa_handler = SIG_DFL;
-                       __libc_sigaction(i, &sa, 0);
+               } else if (sigismember(&hset, i)) {
+                       if (i-32<3U) {
+                               sa.sa_handler = SIG_IGN;
+                       } else {
+                               __libc_sigaction(i, 0, &sa);
+                               if (sa.sa_handler==SIG_IGN) continue;
+                               sa.sa_handler = SIG_DFL;
+                       }
+               } else {
+                       continue;
                }
+               __libc_sigaction(i, &sa, 0);
        }
 
        if (attr->__flags & POSIX_SPAWN_SETPGROUP)

diff --git a/src/signal/sigaction.c b/src/signal/sigaction.c
index 1f09bb967c315fe0e26f62b0d43a694a8e9d24d9..5499bd181c37c06978f4c0a55d9571078b9faffd 100644 (file)
--- a/src/signal/sigaction.c
+++ b/src/signal/sigaction.c
@@ -12,12 +12,26 @@ void __restore(), __restore_rt();
 static pthread_t dummy(void) { return 0; }
 weak_alias(dummy, __pthread_self_def);
 
+static unsigned long handler_set[_NSIG/(8*sizeof(long))];
+
+void __get_handler_set(sigset_t *set)
+{
+       memcpy(set, handler_set, sizeof handler_set);
+}
+
 int __libc_sigaction(int sig, const struct sigaction *restrict sa, struct sigaction *restrict old)
 {
        struct k_sigaction ksa, ksa_old;
+       if (sig >= (unsigned)_NSIG) {
+               errno = EINVAL;
+               return -1;
+       }
        if (sa) {
-               if ((uintptr_t)sa->sa_handler > 1UL)
+               if ((uintptr_t)sa->sa_handler > 1UL) {
+                       a_or_l(handler_set+(sig-1)/(8*sizeof(long)),
+                               1UL<<(sig-1)%(8*sizeof(long)));
                        __pthread_self_def();
+               }
                ksa.handler = sa->sa_handler;
                ksa.flags = sa->sa_flags | SA_RESTORER;
                ksa.restorer = (sa->sa_flags & SA_SIGINFO) ? __restore_rt : __restore;

diff --git a/src/signal/sigismember.c b/src/signal/sigismember.c
index e887b95fd06f599d57940bd6c4fd7ddbb28a5b1a..1a22108b549538d9f3ccdc7a4730c5d51d23bf1a 100644 (file)
--- a/src/signal/sigismember.c
+++ b/src/signal/sigismember.c
@@ -4,9 +4,6 @@
 int sigismember(const sigset_t *set, int sig)
 {
        unsigned s = sig-1;
-       if (s >= 8*sizeof(sigset_t) || sig-32U<3) {
-               errno = EINVAL;
-               return -1;
-       }
+       if (s >= 8*sizeof(sigset_t)) return 0;
        return !!(set->__bits[s/8/sizeof *set->__bits] & 1UL<<(s&8*sizeof *set->__bits-1));
 }