Bug#599382: [patch] Re: wmdrawer: Wmdrawer segfaults when it receives CTRL-C twice.

[Kevin Kreamer]

So, XCloseDisplay doesn't return because the thread that is handling the
signal is blocked by the thread holding all of the X GUI elements (Xlib is
not natively reentrant).  So, the proper solution is to just set a flag in
the signal handler, instead, so that the main event loop knows to end the
program.  The nanosleep was introduced so that we don't block for a long
time on XNextEvent.

Patch attached.

Thanks,
Kevin

On Wed, Apr 6, 2011 at 23:57, Kevin Kreamer <ke...@kreamer.org> wrote:

> I ran into this, as well.
>
> Tracing through the code in gdb, it appears that the call to XCloseDisplay
> on line 125 of wmdrawer.c does not return, causing a SIGTERM not to
> terminate the process.  The second SIGTERM sent then tries to free memory
> previously freed as part of the code handling the first SIGTERM, causing the
> segfault.  Commenting out the call to XCloseDisplay "works" in that it
> allows a CTRL-C to successfully terminate the process, but I don't think
> that's actually the proper solution.
>
> The real question is, why doesn't XCloseDisplay return?
>
> Kevin
>

--- ../orig/wmdrawer-0.10.5/wmdrawer.c	2004-06-26 14:16:04.000000000 -0400
+++ wmdrawer.c	2011-04-12 23:10:17.000000000 -0400
@@ -44,7 +44,8 @@
 #include "pixmaps/defaultHighlightImg.xpm"
 
 static void signalHandler (int signum);
-static void quit (int status);
+static void setExit (int status);
+static void quit (void);
 static void xfreeAllMemory (void);
 static void buildDock (int argc, char *argv[]);
 static void setDockIcon (void);
@@ -80,6 +81,9 @@
 static unsigned int btnDim = 0;
 static int highlightCol, highlightRow;
 
+static int shouldExit;
+static int exitCode;
+
 extern drawerConfig config;
 extern unsigned int nbRows, nbCols, drawerOK;
 extern unsigned int useDefaultIconsBg, useDefaultDockIcon, useDefaultHighlightImg;
@@ -93,6 +97,8 @@
   setDockIcon ();
   buildDrawer ();
   buildTooltip ();
+  exitCode = EXIT_SUCCESS;
+  shouldExit = 0;
   action.sa_handler = signalHandler;
   sigemptyset (&action.sa_mask);
   action.sa_flags = SA_NOCLDSTOP | SA_NODEFER; 
@@ -101,29 +107,34 @@
   sigaction (SIGCHLD, &action, NULL);
 
   eventLoop ();
-  return 0;
+  quit ();
+  return exitCode;
 }
 
 static void signalHandler (int signum) {
   switch (signum) {
   case SIGINT:
   case SIGTERM:
-    quit (EXIT_SUCCESS);
+    setExit(EXIT_SUCCESS);
     break;
   case SIGCHLD:
     while (waitpid (-1, NULL, WNOHANG | WUNTRACED) > 0);
     break;
   default:
-    quit (EXIT_FAILURE);
+    setExit(EXIT_FAILURE);
   }
 }
 
-static void quit (int status) {
+static void setExit (int status) {
+    exitCode = status;
+    shouldExit = 1;
+}
+
+static void quit () {
   printf ("Bye bye, thx to use %s\n", PACKAGE);
   xfreeAllMemory ();
   freeAllMemory ();
   XCloseDisplay (display);
-  exit (status);
 }
 
 void xfreeAllMemory (void) {
@@ -853,8 +864,17 @@
   /* X btn's coord */
   int xbtn = 0, ybtn = 0;
   unsigned int delta, btnIsPressed = 0;
+  struct timespec tv;
+
+  tv.tv_sec = 0;
+  tv.tv_nsec = 100000;
+
+  while (shouldExit == 0) {
+    if (XPending (display) == 0) {
+      nanosleep(&tv, NULL);
+      continue;
+    }
 
-  while (True) {
     XNextEvent (display, &e);
     rebuildApp ();
     /* look at X.h & Xlib.h in /usr/X11R6/include/X11/ */
@@ -1012,7 +1032,7 @@
       }
       break;
     case DestroyNotify:
-      quit (EXIT_SUCCESS);
+      setExit(EXIT_SUCCESS);
     }
   }
 }