lwp: Rewrite signal wait code for portability and safety
authorMarkus Armbruster <armbru@pond.sub.org>
Sun, 27 Dec 2020 08:36:54 +0000 (09:36 +0100)
committerMarkus Armbruster <armbru@pond.sub.org>
Sun, 17 Jan 2021 20:24:28 +0000 (21:24 +0100)
LWP's use of sigset_t is problematic.

To iterate over a sigset_t, it uses NSIG, which is not portable: BSD
and System V provide it, but it's not POSIX.

To record signals caught, it updates a sigset_t variable from a signal
handler.  The variable isn't volatile, because we'd have to cast away
volatile for sigaddset().

Replace sigset_t by an array of signal numbers terminated with 0.

Since lwpInitSigWait() needs to store the signal set for
lwpCatchAwaitedSig() anyway, there is no need to pass it to
lwpSigWait().  Drop its parameter.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
include/lwp.h
src/lib/empthread/lwp.c
src/lib/lwp/lwp.c
src/lib/lwp/lwpint.h
src/lib/lwp/sig.c

index 1a96322c67dc0622561f3497ff53a3fa9f393014..e115ad704446b41f2788b521a6c9be52cf2d54cc 100644 (file)
@@ -28,7 +28,7 @@
  *  lwp.h -- prototypes and structures for lightweight processes
  *
  *  Known contributors to this file:
- *     Markus Armbruster, 2004-2013
+ *     Markus Armbruster, 2004-2020
  *     Ron Koenderink, 2007-2009
  */
 
@@ -41,7 +41,6 @@
 #ifndef LWP_H
 #define LWP_H
 
-#include <signal.h>
 #include <sys/time.h>
 
 #define LWP_STACKCHECK 0x1
@@ -55,7 +54,7 @@ struct lwp_rwlock;
 
 #define LWP_MAX_PRIO   8
 
-struct lwpProc *lwpInitSystem(int prio, void **ctxp, int flags, sigset_t *);
+struct lwpProc *lwpInitSystem(int prio, void **ctxp, int flags, int[]);
 struct lwpProc *lwpCreate(int prio, void (*)(void *), int size,
                          int flags, char *name,
                          int argc, char **argv, void *ud);
@@ -65,7 +64,7 @@ void lwpYield(void);
 int lwpSleepFd(int fd, int flags, struct timeval *timeout);
 int lwpSleepUntil(time_t until);
 void lwpWakeup(struct lwpProc *);
-int lwpSigWait(sigset_t *set, int *sig);
+int lwpSigWait(int *sig);
 void *lwpGetUD(struct lwpProc *);
 void lwpSetUD(struct lwpProc *, char *ud);
 int lwpSetPriority(int prio);
index 376fc62299efad6de290e036b82a0ba187b14361..d26eae7c889c03c2325e12a55841f2df46973730 100644 (file)
@@ -28,7 +28,7 @@
  *
  *  Known contributors to this file:
  *     Sasha Mikheev
- *     Markus Armbruster, 2006-2009
+ *     Markus Armbruster, 2006-2020
  */
 
 #include <config.h>
 /* Flags that were passed to empth_init() */
 static int empth_flags;
 
-
 int
 empth_init(void **ctx, int flags)
 {
-    sigset_t set;
-
+    static int sig[] = { SIGHUP, SIGINT, SIGTERM, 0 };
     empth_flags = flags;
     empth_init_signals();
-    sigemptyset(&set);
-    sigaddset(&set, SIGHUP);
-    sigaddset(&set, SIGINT);
-    sigaddset(&set, SIGTERM);
-    lwpInitSystem(1, ctx, flags, &set);
+    lwpInitSystem(1, ctx, flags, sig);
     return 0;
 }
 
-
 empth_t *
 empth_create(void (*entry)(void *), int size, int flags,
             char *name, void *ud)
@@ -124,17 +117,12 @@ empth_sleep(time_t until)
 int
 empth_wait_for_signal(void)
 {
-    sigset_t set;
     int sig, err;
     time_t now;
 
     ef_make_stale();
-    sigemptyset(&set);
-    sigaddset(&set, SIGHUP);
-    sigaddset(&set, SIGINT);
-    sigaddset(&set, SIGTERM);
     for (;;) {
-       err = lwpSigWait(&set, &sig);
+       err = lwpSigWait(&sig);
        if (CANT_HAPPEN(err)) {
            time(&now);
            lwpSleepUntil(now + 60);
index 803aff0917038773e3189fd926ad134056947373..a7d33b980e8e129c70277a8454810848eae76cd7 100644 (file)
@@ -28,7 +28,7 @@
  *  lwp.c: lightweight process creation, destruction and manipulation
  *
  *  Known contributors to this file:
- *     Markus Armbruster, 2004-2017
+ *     Markus Armbruster, 2004-2020
  */
 
 #include <config.h>
@@ -296,7 +296,7 @@ lwpSetPriority(int new)
  * initialise the coroutine structures
  */
 struct lwpProc *
-lwpInitSystem(int pri, void **ctxptr, int flags, sigset_t *waitset)
+lwpInitSystem(int pri, void **ctxptr, int flags, int sig[])
 {
     struct lwpQueue *q;
     int i, *stack, marker;
@@ -323,7 +323,7 @@ lwpInitSystem(int pri, void **ctxptr, int flags, sigset_t *waitset)
     for (i = LWP_MAX_PRIO, q = LwpSchedQ; i--; q++)
        q->head = q->tail = NULL;
     LwpDeadQ.head = LwpDeadQ.tail = NULL;
-    lwpInitSigWait(waitset);
+    lwpInitSigWait(sig);
     /* must be lower in priority than us for this to work right */
     sel = lwpCreate(0, lwpSelect, 65536, flags, "EventHandler", 0,
                    NULL, NULL);
index cca3eac7fde6f6c2c0374dcc2045bc8e33ea933c..7a317140f85efaa67c7091044efa079769bd51f3 100644 (file)
  *  lwpint.h: lwp internal structures
  *
  *  Known contributors to this file:
- *     Markus Armbruster, 2004-2009
+ *     Markus Armbruster, 2004-2020
  */
 
 #ifndef LWPINT_H
 #define LWPINT_H
 
-#include <signal.h>
 #include <time.h>
 #include <ucontext.h>
 
@@ -82,7 +81,7 @@ void lwpEntryPoint(void);
 void lwpInitSelect(struct lwpProc *);
 void lwpWakeupSleep(void);
 void lwpSelect(void *);
-void lwpInitSigWait(sigset_t *);
+void lwpInitSigWait(int[]);
 void lwpSigWakeup(void);
 void lwpStatus(struct lwpProc *, char *, ...)
     ATTRIBUTE((format (printf, 2, 3)));
index 62d19728af5cbfadfdc7733c3a4bfd3d817e2e12..41d1c31eb4132ac267261193e714b1d2da1f6ae3 100644 (file)
 
 #include <errno.h>
 #include <stddef.h>
+#include <stdlib.h>
 #include <signal.h>
 #include "lwp.h"
 #include "lwpint.h"
 
 /*
- * Signals caught so far.
+ * Awaited signal numbers, terminated with 0.
+ */
+static int *LwpAwaitedSig;
+
+/*
+ * Pending awaited signals.
  * Access only with signals blocked!
  */
-static sigset_t LwpSigCaught;
+static volatile int *LwpSigCaught;
 
 /*
- * LwpSigCaught changed since last
+ * Is there anything in LwpSigCaught[]?
  */
-static sig_atomic_t LwpSigCheck;
+static volatile sig_atomic_t LwpSigCheck;
 
 /* The thread waiting for signals in lwpSigWait() */
 static struct lwpProc *LwpSigWaiter;
@@ -55,80 +61,96 @@ static struct lwpProc *LwpSigWaiter;
 static void lwpCatchAwaitedSig(int);
 
 /*
- * Initialize waiting for signals in @set.
+ * Initialize waiting for signals in @sig[].
+ * @sig[] contains signal numbers, terminated with 0.  It must have
+ * static storage duration.
  */
 void
-lwpInitSigWait(sigset_t *set)
+lwpInitSigWait(int sig[])
 {
     struct sigaction act;
     int i;
 
-    sigemptyset(&LwpSigCaught);
+    LwpAwaitedSig = sig;
 
     act.sa_flags = 0;
-    act.sa_mask = *set;
     act.sa_handler = lwpCatchAwaitedSig;
-    for (i = 0; i < NSIG; i++) {
-       if (sigismember(set, i) > 0)
-           sigaction(i, &act, NULL);
-    }
+    sigemptyset(&act.sa_mask);
+    for (i = 0; sig[i]; i++)
+       sigaddset(&act.sa_mask, sig[i]);
+
+    LwpSigCaught = calloc(i, sizeof(*LwpSigCaught));
+
+    for (i = 0; sig[i]; i++)
+       sigaction(sig[i], &act, NULL);
 }
 
+/*
+ * Signal handler for awaited signals.
+ * Set @LwpSigCaught[] for @sig, and set @LwpSigCheck.
+ * Not reentrant; lwpInitSigWait() guards.
+ */
 static void
 lwpCatchAwaitedSig(int sig)
 {
-    sigaddset(&LwpSigCaught, sig);
+    int i;
+
+    for (i = 0; LwpAwaitedSig[i]; i++) {
+       if (sig == LwpAwaitedSig[i])
+           LwpSigCaught[i] = 1;
+    }
     LwpSigCheck = 1;
 }
 
 /*
- * Test whether a signal from @set has been caught.
- * If yes, delete that signal from the set of caught signals, and
+ * Test whether an awaited signal is pending.
+ * If yes, remove that signal from the set of pending signals, and
  * return its number.
  * Else return 0.
  */
 static int
-lwpGetSig(sigset_t *set)
+lwpGetSig(void)
 {
-    sigset_t save;
-    int i, j;
+    int ret = 0;
+    sigset_t set, save;
+    int i;
 
-    sigprocmask(SIG_BLOCK, set, &save);
+    sigemptyset(&set);
+    for (i = 0; LwpAwaitedSig[i]; i++)
+       sigaddset(&set, LwpAwaitedSig[i]);
+    sigprocmask(SIG_BLOCK, &set, &save);
 
-    for (i = NSIG - 1; i > 0; i--) {
-       if (sigismember(set, i) > 0 && sigismember(&LwpSigCaught, i) > 0) {
-           lwpStatus(LwpCurrent, "Got awaited signal %d", i);
-           sigdelset(&LwpSigCaught, i);
-           break;
+    for (i = 0; LwpAwaitedSig[i]; i++) {
+       if (LwpSigCaught[i]) {
+           lwpStatus(LwpCurrent, "Got awaited signal %d", LwpSigCaught[i]);
+           ret = LwpAwaitedSig[i];
+           LwpSigCaught[i] = 0;
        }
     }
 
-    for (j = i;
-        sigismember(set, i) > 0 && sigismember(&LwpSigCaught, i) > 0;
-        j--)
-       ;
-    if (!j)
+    for (; LwpAwaitedSig[i] && LwpSigCaught[i]; i++) ;
+    if (!LwpSigCaught[i])
        LwpSigCheck = 0;
 
     sigprocmask(SIG_SETMASK, &save, NULL);
-    return i;
+    return ret;
 }
 
 /*
- * Wait until a signal from @set arrives.
+ * Wait until one of the signals passed to lwpInitSigWait() arrives.
  * Assign its number to *@sig and return 0.
  * If another thread is already waiting for signals, return EBUSY
  * without waiting.
  */
 int
-lwpSigWait(sigset_t *set, int *sig)
+lwpSigWait(int *sig)
 {
     int res;
 
     if (LwpSigWaiter)
        return EBUSY;
     for (;;) {
-       res = lwpGetSig(set);
+       res = lwpGetSig();
        if (res > 0)
            break;
        lwpStatus(LwpCurrent, "Waiting for signals");