From: Rodney Padgett Date: Sat, 9 Aug 2008 10:39:24 +0000 (+0100) Subject: Use inotify to check for changes to the defaults database. Workaround for X-Git-Tag: wmaker-0.93.0-crm~57 X-Git-Url: https://repo.or.cz/w/wmaker-crm.git/commitdiff_plain/56d856878743ec2d3b8d98ab6a0b61a6b2c99129 Use inotify to check for changes to the defaults database. Workaround for 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. --- diff --git a/INSTALL b/INSTALL index 91fd3cbe..5aa4ef61 100644 --- 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: diff --git a/src/event.c b/src/event.c index a4f614f4..7adaba95 100644 --- a/src/event.c +++ b/src/event.c @@ -20,7 +20,7 @@ * USA. */ - +#include #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 -- diff --git a/src/main.c b/src/main.c index c849de3c..b61ad70e 100644 --- a/src/main.c +++ b/src/main.c @@ -19,9 +19,7 @@ * USA. */ -#define _GNU_SOURCE /* needed to get the defines in glibc 2.2 */ -#include /* this has the needed values defined */ -#include +#include #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; } diff --git a/src/shutdown.c b/src/shutdown.c index 5beec3d6..d9a3caaa 100644 --- a/src/shutdown.c +++ b/src/shutdown.c @@ -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; ihelper_pid) diff --git a/src/startup.c b/src/startup.c index 12d001de..af0c8e30 100644 --- a/src/startup.c +++ b/src/startup.c @@ -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;