plugins: pidfile: tricky zebra doesn't close its pid file

This patch fixes a few really hard problems wrt PID files:

 1. Listening for IN_CREATE events means we get notified immediately by
    the kernel when someone calls open()/fopen() on a PID file.  Reading
    the contents returns 0, thank you atoi() ... so we drop IN_CREATE
    and instead look for IN_CLOSE_WRITE, there fixed it!  Not quite ...

 2. Some programs, like Zebra, and other Quagga/Frr daemons, don't
    close() their PID files after creation.  Instead they ftruncate()
    and keep them open, and locked.  Presumably to get a mechanism to
    detect already running instances -- messes life up a bit for the
    rest of us though.  So we need to read files after IN_MODIFY too.

 3. We also want to track IN_DELETE so we can deassert conditions when
    services exit gracefully and clean up their PID files

The rest fo the commit is debug instrumentation changes.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
This commit is contained in:
Joachim Wiberg committed 2021-03-11 11:44:23 +01:00
1 parent 05ac9c18c9
commit 2711b27971
2 files changed
+8 -13

No files matched your search

+5 -12
View File
@@ -58,7 +58,7 @@ static int pidfile_add_path(struct iwatch *iw, char *path)
} }
} }
return iwatch_add(iw, path, IN_ONLYDIR); return iwatch_add(iw, path, IN_ONLYDIR | IN_CLOSE_WRITE);
} }
static void pidfile_update_conds(char *dir, char *name, uint32_t mask) static void pidfile_update_conds(char *dir, char *name, uint32_t mask)
@@ -68,11 +68,11 @@ static void pidfile_update_conds(char *dir, char *name, uint32_t mask)
svc_t *svc; svc_t *svc;
paste(fn, sizeof(fn), dir, name); paste(fn, sizeof(fn), dir, name);
_d("path: %s, mask: %08x", fn, mask);
if (fnmatch("*\\.pid", fn, 0) && fnmatch("*/pid", fn, 0)) if (fnmatch("*\\.pid", fn, 0) && fnmatch("*/pid", fn, 0))
return; return;
_d("path: %s, mask: %08x", fn, mask);
svc = svc_find_by_pidfile(fn); svc = svc_find_by_pidfile(fn);
if (!svc) { if (!svc) {
_d("No matching svc for %s", fn); _d("No matching svc for %s", fn);
@@ -82,7 +82,7 @@ static void pidfile_update_conds(char *dir, char *name, uint32_t mask)
_d("Found svc %s for %s with pid %d", svc->name, fn, svc->pid); _d("Found svc %s for %s with pid %d", svc->name, fn, svc->pid);
mkcond(svc, cond, sizeof(cond)); mkcond(svc, cond, sizeof(cond));
if (mask & (IN_CREATE | IN_ATTRIB | IN_MODIFY | IN_MOVED_TO)) { if (mask & (IN_CLOSE_WRITE | IN_ATTRIB | IN_MODIFY | IN_MOVED_TO)) {
svc_started(svc); svc_started(svc);
if (svc_is_forking(svc)) { if (svc_is_forking(svc)) {
pid_t pid; pid_t pid;
@@ -174,8 +174,6 @@ static void pidfile_callback(void *arg, int fd, int events)
if (!ev->mask) if (!ev->mask)
continue; continue;
_d("name %s, event: 0x%08x", ev->name, ev->mask);
/* Find base path for this event */ /* Find base path for this event */
iwp = iwatch_find_by_wd(&iw_pidfile, ev->wd); iwp = iwatch_find_by_wd(&iw_pidfile, ev->wd);
if (!iwp) if (!iwp)
@@ -186,12 +184,7 @@ static void pidfile_callback(void *arg, int fd, int events)
continue; continue;
} }
if (ev->mask & IN_DELETE) { if (ev->mask & (IN_CLOSE_WRITE | IN_DELETE | IN_ATTRIB | IN_MODIFY | IN_MOVED_TO))
_d("pidfile %s/%s removed ...", iwp->path, ev->name);
continue;
}
if (ev->mask & (IN_CREATE | IN_ATTRIB | IN_MODIFY | IN_MOVED_TO))
pidfile_update_conds(iwp->path, ev->name, ev->mask); pidfile_update_conds(iwp->path, ev->name, ev->mask);
} }
} }
+3 -1
View File
@@ -415,8 +415,10 @@ svc_t *svc_find_by_pidfile(char *fn)
pid_t pid; pid_t pid;
pid = pid_file_read(fn); pid = pid_file_read(fn);
if (pid == -1) if (pid <= 0) {
_d("pid_file_read(%s) => %d", fn, pid);
return NULL; return NULL;
}
for (svc = svc_iterator(&iter, 1); svc; svc = svc_iterator(&iter, 0)) { for (svc = svc_iterator(&iter, 1); svc; svc = svc_iterator(&iter, 0)) {
if (svc->pid != pid) if (svc->pid != pid)