Use inotify to check for changes to the defaults database. Workaround for
authorRodney Padgett <rod_padgett@hotmail.com>
Sat, 9 Aug 2008 10:39:24 +0000 (9 11:39 +0100)
committerCarlos R. Mafra <crmafra@gmail.com>
Sat, 9 Aug 2008 18:14:39 +0000 (9 13:14 -0500)
event handler timer.

After upgrading my kernel recently I noticed that dnotify has been
depreciated, so I decided to try and implement the new inotify code in
Window Maker instead.

During testing, I also found that one of the timers which was removed
(the one causing the most wake-ups), calling delayedAction, was
responsible for handling signals. Basically with this timer removed,
signals were only handled after an X event occurs.

After looking at the delayedAction function, I couldn't see the purpose of it.
It certainly wouldn't cause any delay as it was called by the timer every
500ms, so there is no time correlation with when a signal was received.
Also, it appeared to count the signals and call DispatchEvent for each
one, but it appears DispatchEvent would just handle the most recent signal
and take action on that. The signals handled by delayedAction are the
various exit and reset signals, so only one need to be handled. I
therefore have commented out delayedAction (it wasn't called by any other
procedure) and added a call to DispatchEvent imediately after the signal
is registered, in handleExitSig.

I'm not sure what problems this may cause with dead children - these are
only cleaned up after an Xevent now that the timer is removed- but I
haven't observed any problems since a few months ago.

INSTALL
src/event.c
src/main.c
src/shutdown.c
src/startup.c

diff --git a/INSTALL b/INSTALL
index 91fd3cb..5aa4ef6 100644 (file)
--- a/INSTALL
+++ b/INSTALL
@@ -120,6 +120,12 @@ might work too.
 CONFIGURE OPTIONS:
 ==================
 
+If you downloaded the cvs or git versions, type
+
+./autogen.sh
+
+to generate the config files.
+
 These options can be passed to the configure script to enable/disable
 some Window Maker features. Example:
 
index a4f614f..7adaba9 100644 (file)
@@ -20,7 +20,7 @@
  *  USA.
  */
 
-
+#include <sys/inotify.h>
 #include "wconfig.h"
 
 
@@ -100,12 +100,11 @@ extern int wXkbEventBase;
 #endif
 
 /* special flags */
-extern char WDelayedActionSet;
+/*extern char WDelayedActionSet;*/
 
 
 /************ Local stuff ***********/
 
-
 static void saveTimestamp(XEvent *event);
 static void handleColormapNotify();
 static void handleMapNotify();
@@ -302,6 +301,69 @@ DispatchEvent(XEvent *event)
     }
 }
 
+/*
+ *----------------------------------------------------------------------
+ * inotifyHandleEvents-
+ *     Check for inotify events
+ *
+ * Returns:
+ *     After reading events for the given file descriptor (fd) and
+ *     watch descriptor (wd)
+ *
+ * Side effects:
+ *     Calls wDefaultsCheckDomains if config database is updated
+ *----------------------------------------------------------------------
+ */
+
+/* Allow for 1024 simultanious events */
+#define BUFF_SIZE ((sizeof(struct inotify_event)+FILENAME_MAX)*1024)
+void inotifyHandleEvents (int fd, int wd)
+{
+    ssize_t eventQLength, i = 0;
+    char buff[BUFF_SIZE] = {0};
+    extern void wDefaultsCheckDomains();
+    int oneShotFlag=0; /* Only check config once per read of the event queue */
+
+    /* Read off the queued events
+     * queue overflow is not checked (IN_Q_OVERFLOW). In practise this should
+     * not occur; the block is on Xevents, but a config file change will normally
+     * occur as a result of an Xevent - so the event queue should never have more than
+     * a few entries before a read().
+     */
+    eventQLength = read (fd, buff, BUFF_SIZE);
+    
+    /* check what events occured */
+    /* Should really check wd here too, but for now we only have one watch! */
+    while (i < eventQLength) {
+        struct inotify_event *pevent = (struct inotify_event *)&buff[i];
+       
+        /*
+         * see inotify.h for event types.
+         */
+        if (pevent->mask & IN_DELETE_SELF) {
+            wwarning(_("the defaults database has been deleted!"
+                       " Restart Window Maker to create the database"
+                       " with the default settings"));
+            close(fd);
+        }
+        if (pevent->mask & IN_UNMOUNT) {
+            wwarning(_("the unit containing the defaults database has"
+                       " been unmounted. Setting --static mode."
+                       " Any changes will not be saved."));
+            close(fd);
+            wPreferences.flags.noupdates=1;
+        }
+        if ((pevent->mask & IN_MODIFY) && oneShotFlag == 0) {
+           fprintf(stdout,"wmaker: reading config files in defaults database.\n");
+            wDefaultsCheckDomains(NULL);
+        }
+        /* Check for filename (length of name (len) > 0) */
+        /* if (pevent->len) printf ("name=%s\n", pevent->name); */
+      
+         i += sizeof(struct inotify_event) + pevent->len; /* move to next event in the buffer */
+   }
+
+}  /* inotifyHandleEvents */
 
 /*
  *----------------------------------------------------------------------
@@ -313,32 +375,49 @@ DispatchEvent(XEvent *event)
  *
  * Side effects:
  *     The LastTimestamp global variable is updated.
+ *      Calls inotifyGetEvents if defaults database changes.
  *----------------------------------------------------------------------
  */
 void
 EventLoop()
 {
     XEvent event;
-    extern volatile int filesChanged;
-    extern void wDefaultsCheckDomains();
+    extern int inotifyFD;
+    extern int inotifyWD;
+    struct timeval time;
+    fd_set rfds;
+    int retVal=0;
+    
+    if (inotifyFD < 0 || inotifyWD < 0)
+        retVal = -1;
 
     for(;;) {
-        WMNextEvent(dpy, &event);
-        WMHandleEvent(&event);
 
-       /*
-        * If dnotify detects changes in configuration
-        * files we have to read them again.
-        */
-       if (filesChanged){
-               fprintf(stdout,"wmaker: reading config files in defaults database.\n");
-               wDefaultsCheckDomains(NULL);
-               filesChanged = 0;
-       }
+         WMNextEvent(dpy, &event); /* Blocks here */
+         WMHandleEvent(&event);
+
+         if (retVal != -1 ) {
+            time.tv_sec = 0;
+            time.tv_usec = 0;
+            FD_ZERO (&rfds);
+            FD_SET (inotifyFD, &rfds);
+
+            /* check for available read data from inotify - don't block! */
+            retVal = select (inotifyFD + 1, &rfds, NULL, NULL, &time);
+
+            if (retVal < 0) { /* an error has occured */
+                wwarning(_("select failed. The inotify instance will be closed."
+                           " Changes to the defaults database will require"
+                           " a restart to take effect."));
+                close(inotifyFD);
+                continue;
+            }
+            if (FD_ISSET (inotifyFD, &rfds))
+                  inotifyHandleEvents(inotifyFD,inotifyWD);
+         }
     }
 }
 
-
 /*
  *----------------------------------------------------------------------
  * ProcessPendingEvents --
index c849de3..b61ad70 100644 (file)
@@ -19,9 +19,7 @@
  *  USA.
  */
 
-#define _GNU_SOURCE    /* needed to get the defines in glibc 2.2 */
-#include <fcntl.h>     /* this has the needed values defined */
-#include <signal.h>
+#include <sys/inotify.h>
 
 #include "wconfig.h"
 
@@ -66,8 +64,9 @@ Display       *dpy;
 char *ProgName;
 
 unsigned int ValidModMask = 0xff;
-volatile int filesChanged;
 
+int inotifyFD;
+int inotifyWD;
 /* locale to use. NULL==POSIX or C */
 char *Locale=NULL;
 
@@ -495,33 +494,41 @@ check_defaults()
 
 
 /*
- * This is the handler used to notify if configuration
- * files have changed, using linux kernel'l dnotify
- * mechanism (from Documentation/dnotify.txt)
+ * Add watch here, used to notify if configuration
+ * files have changed, using linux kernel inotify mechanism
  */
-void handler(int sig, siginfo_t *si, void *data)
+
+static void
+inotifyWatchConfig()
 {
-       filesChanged = si->si_fd;
+    char *watchPath;
+    inotifyFD = inotify_init(); /* Initialise an inotify instance */
+    if (inotifyFD < 0) {
+         wwarning(_("could not initialise an inotify instance."
+                   " Changes to the defaults database will require"
+                   " a restart to take effect. Check your kernel!"));
+    } else {           
+         watchPath = wstrconcat(wusergnusteppath(), "/Defaults");
+         /* Add the watch; really we are only looking for modify events
+         * but we might want more in the future so check all events for now.
+         * The individual events are checked for in event.c.
+         */
+         inotifyWD = inotify_add_watch (inotifyFD, watchPath, IN_ALL_EVENTS);
+         if (inotifyWD < 0) {
+            wwarning(_("could not add an inotify watch on path\n."
+                        "%s\n"
+                        "Changes to the defaults database will require"
+                        " a restart to take effect."),watchPath);
+            close (inotifyFD);
+         }
+    }
+    wfree(watchPath);
 }
 
 static void
 execInitScript()
 {
-       char *file, *path, *paths;
-       struct sigaction act;
-       volatile int fd;
-
-       path = wstrconcat(wusergnusteppath(), "/Defaults");
-
-       act.sa_sigaction = handler;
-       sigemptyset(&act.sa_mask);
-       act.sa_flags = SA_SIGINFO;
-       sigaction(SIGRTMIN + 1, &act, NULL);
-
-       fd = open(path, O_RDONLY);
-       fcntl(fd, F_SETSIG, SIGRTMIN + 1);
-       fcntl(fd, F_NOTIFY, DN_MODIFY|DN_MULTISHOT);
-       wfree(path);
+    char *file, *paths;
 
     paths = wstrconcat(wusergnusteppath(), "/Library/WindowMaker");
     paths = wstrappend(paths, ":"DEF_CONFIG_PATHS);
@@ -897,7 +904,7 @@ real_main(int argc, char **argv)
         multiHead = False;
 
     execInitScript();
-
+    inotifyWatchConfig();
     EventLoop();
     return -1;
 }
index 5beec3d..d9a3caa 100644 (file)
@@ -59,6 +59,7 @@ void
 Shutdown(WShutdownMode mode)
 {
     int i;
+    extern int inotifyFD;
 
     switch (mode) {
     case WSLogoutMode:
@@ -85,6 +86,7 @@ Shutdown(WShutdownMode mode)
                 }
             }
 #endif
+        close(inotifyFD);
         for (i = 0; i < wScreenCount; i++) {
             WScreen *scr;
 
@@ -115,6 +117,7 @@ Shutdown(WShutdownMode mode)
         for (i=0; i<wScreenCount; i++) {
             WScreen *scr;
 
+            close(inotifyFD);
             scr = wScreenWithNumber(i);
             if (scr) {
                 if (scr->helper_pid)
index 12d001d..af0c8e3 100644 (file)
@@ -140,7 +140,7 @@ extern Atom _XA_GNUSTEP_TITLEBAR_STATE;
 extern Cursor wCursor[WCUR_LAST];
 
 /* special flags */
-extern char WDelayedActionSet;
+/*extern char WDelayedActionSet;*/
 
 /***** Local *****/
 
@@ -216,8 +216,10 @@ handleXIO(Display *xio_dpy)
  *----------------------------------------------------------------------
  * delayedAction-
  *     Action to be executed after the signal() handler is exited.
+ * This was called every 500ms to 'clean up' signals. Not used now.
  *----------------------------------------------------------------------
  */
+#ifdef notused
 static void
 delayedAction(void *cdata)
 {
@@ -232,7 +234,7 @@ delayedAction(void *cdata)
      */
     DispatchEvent(NULL);
 }
-
+#endif
 
 /*
  *----------------------------------------------------------------------
@@ -256,10 +258,7 @@ handleExitSig(int sig)
 #endif
 
         SIG_WCHANGE_STATE(WSTATE_NEED_RESTART);
-        /* setup idle handler, so that this will be handled when
-         * the select() is returned becaused of the signal, even if
-         * there are no X events in the queue */
-        WDelayedActionSet++;
+
     } else if (sig == SIGUSR2) {
 #ifdef SYS_SIGLIST_DECLARED
         wwarning("got signal %i (%s) - rereading defaults\n", sig, sys_siglist[sig]);
@@ -268,10 +267,7 @@ handleExitSig(int sig)
 #endif
 
         SIG_WCHANGE_STATE(WSTATE_NEED_REREAD);
-        /* setup idle handler, so that this will be handled when
-         * the select() is returned becaused of the signal, even if
-         * there are no X events in the queue */
-        WDelayedActionSet++;
+
     } else if (sig==SIGTERM || sig==SIGINT || sig==SIGHUP) {
 #ifdef SYS_SIGLIST_DECLARED
         wwarning("got signal %i (%s) - exiting...\n", sig, sys_siglist[sig]);
@@ -281,10 +277,10 @@ handleExitSig(int sig)
 
         SIG_WCHANGE_STATE(WSTATE_NEED_EXIT);
 
-        WDelayedActionSet++;
     }
 
     sigprocmask(SIG_UNBLOCK, &sigs, NULL);
+    DispatchEvent(NULL); /* Dispatch events imediately.*/
 }
 
 /* Dummy signal handler */
@@ -348,8 +344,6 @@ buryChild(int foo)
         NotifyDeadProcess(pid, WEXITSTATUS(status));
     }
 
-    WDelayedActionSet++;
-
     sigprocmask(SIG_UNBLOCK, &sigs, NULL);
 
     errno = save_errno;