Clean thread/spawn/signal usage in wmfs

- Don't create 2 thread
- Don't create status thread when conf.status_timing == 0
- Don't launch status script if still running
- Remove double fork() hack in spawn()
- Wait childs properly
- mutex on conf.status_pid and sig_chld variable, thread-safe and
  signal-safe
- Set exiting variable volatile, signal-safe
- Use sigaction instead of signal

TODO: set mutex for exiting variable because this is not thread-safe

src/util.c

@@ -253,14 +253,13 @@ alias_to_str(char *conf_choice)
  * \param cmd Command
  * \return child pid
 */
-int
+pid_t
 spawn(const char *format, ...)
 {
      char *sh = NULL;
      char cmd[512];
      va_list ap;
-     pid_t pid, ret;
+     pid_t pid;
-     int p[2];
      size_t len;
 
      va_start(ap, format);
@@ -276,48 +275,19 @@ spawn(const char *format, ...)
      if(!(sh = getenv("SHELL")))
           sh = "/bin/sh";
 
-     if (pipe(p) == -1)
-     {
-          warn("pipe");
-          return -1;
-     }
-
      if((pid = fork()) == 0)
      {
-          close(p[0]);
+          if(dpy)
-          if((pid = fork()) == 0)
+               close(ConnectionNumber(dpy));
-          {
+          setsid();
-               if(dpy)
+          if (execl(sh, sh, "-c", cmd, (char*)NULL) == -1)
-                    close(ConnectionNumber(dpy));
+               warn("execl(sh -c %s)", cmd);
-               setsid();
+          exit(EXIT_FAILURE);
-               execl(sh, sh, "-c", cmd, (char*)NULL);
-               exit(EXIT_FAILURE);
-          }
-
-          if (sizeof(pid_t) != write(p[1], &pid, sizeof(pid_t)))
-               warn("write");
-
-          close(p[1]);
-          exit(EXIT_SUCCESS);
      }
-     else if (pid != -1)
+     else if (pid == -1)
-     {
-          close(p[1]);
-          if (sizeof(pid_t) != read(p[0], &ret, sizeof(pid_t)))
-          {
-               warn("read");
-               ret = -1;
-          }
-          close(p[0]);
-          waitpid(pid, NULL, 0);
-     }
-     else
-     {
           warn("fork");
-          ret = -1;
-     }
 
-     return ret;
+     return pid;
 }
 
 /** Swap two pointer.

src/wmfs.c

@@ -32,6 +32,9 @@
 
 #include "wmfs.h"
 
+static volatile Bool exiting = False, sig_chld = False;
+static pthread_mutex_t mtx = PTHREAD_MUTEX_INITIALIZER;
+
 static void signal_handle(int);
 
 int
@@ -141,27 +144,43 @@ quit(void)
      return;
 }
 
-void *
+static void
-thread_process(void *arg)
+wait_childs_and_status(void)
 {
-     XEvent ev;
+     int pid;
 
-     /* X event loop */
+     pthread_mutex_lock(&mtx);
-     if(arg)
+     if (sig_chld) {
-     {
+          while ((pid = waitpid(-1, NULL, WNOHANG)) > 0)
-          while(!exiting && !XNextEvent(dpy, &ev))
+               if (pid == conf.status_pid)
-               getevent(ev);
+                    conf.status_pid = -1;
+          sig_chld = False;
      }
-     /* Status checking loop with timing */
+     pthread_mutex_unlock(&mtx);
-     else
+}
+
+void *
+thread_status(void *arg)
+{
+     (void)arg;
+     int left = conf.status_timing;
+
+     pthread_detach(pthread_self());
+
+     do
      {
-          pthread_detach(pthread_self());
+          wait_childs_and_status();
-          do
+
-          {
+          pthread_mutex_lock(&mtx);
+          if (conf.status_pid == -1)
                conf.status_pid = spawn(conf.status_path);
-               sleep(conf.status_timing);
+          pthread_mutex_unlock(&mtx);
-          } while (!exiting && conf.status_timing != 0);
+
-     }
+          while ((left = sleep(left)) > 0);
+          left = conf.status_timing;
+
+     } while (!exiting);
+
      pthread_exit(NULL);
 }
 
@@ -171,20 +190,23 @@ void
 mainloop(void)
 {
      XEvent ev;
-     pthread_t evloop, evstatus;
+     pthread_t th_status;
-     void *ret;
 
-     if(!estatus)
+     if (estatus && conf.status_timing == 0)
-          while(!exiting && !XNextEvent(dpy, &ev))
+          conf.status_pid = spawn(conf.status_path);
-               getevent(ev);
+     else if (estatus && pthread_create(&th_status, NULL, thread_status, NULL) != 0) {
-     else
+          warnx("pthread_create");
-     {
+          estatus = False;
-          pthread_create(&evloop, NULL, thread_process, "1");
-          pthread_create(&evstatus, NULL, thread_process, NULL);
-
-          (void)pthread_join(evloop, &ret);
      }
 
+     while (!exiting && !XNextEvent(dpy, &ev)) {
+          getevent(ev);
+          wait_childs_and_status();
+     }
+
+     if (estatus)
+          pthread_join(th_status, NULL);
+
      return;
 }
 
@@ -409,14 +431,9 @@ signal_handle(int sig)
           case SIGQUIT:
           case SIGTERM:
                exiting = True;
-               quit();
-               exit(EXIT_SUCCESS);
                break;
           case SIGCHLD:
-               /* re-set signal handler and wait childs */
+               sig_chld = True;
-               if (signal(SIGCHLD, &signal_handle) == SIG_ERR)
-                    warn("signal(%d)", SIGCHLD);
-               while (waitpid(-1, NULL, WNOHANG) > 0);
                break;
      }
 
@@ -435,7 +452,7 @@ main(int argc, char **argv)
      char *ol = "csgVS";
      extern char *optarg;
      extern int optind;
-     int sigs[] = { SIGTERM, SIGQUIT, SIGCHLD };
+     struct sigaction sa;
 
      argv_global  = xstrdup(argv[0]);
      all_argv = argv;
@@ -518,9 +535,12 @@ main(int argc, char **argv)
           errx(EXIT_FAILURE, "cannot open X server.");
 
      /* Set signal handler */
-     for (i = sigs[0]; i < (int)LEN(sigs); i++)
+     memset(&sa, 0, sizeof(sa));
-          if (signal(sigs[i], &signal_handle) == SIG_ERR)
+     sa.sa_handler = signal_handle;
-               warn("signal(%d)", sigs[i]);
+     sigemptyset(&sa.sa_mask);
+     sigaction(SIGQUIT, &sa, NULL);
+     sigaction(SIGTERM, &sa, NULL);
+     sigaction(SIGCHLD, &sa, NULL);
 
      /* Check if an other WM is already running; set the error handler */
      XSetErrorHandler(errorhandler);

src/wmfs.h

@@ -431,7 +431,7 @@ int selscreen;
 int prevselscreen;
 Conf conf;
 Key *keys;
-Bool exiting, estatus;
+Bool estatus;
 XRectangle *sgeo;
 XRectangle *spgeo;
 Cursor cursor[CurLast];