From 2711b279714dfa56ef77a1a1d74d58f461fbce9f Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Thu, 11 Mar 2021 11:32:09 +0100 Subject: [PATCH] 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 --- plugins/pidfile.c | 17 +++++------------ src/svc.c | 4 +++- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/plugins/pidfile.c b/plugins/pidfile.c index 50929073..dfb3b7df 100644 --- a/plugins/pidfile.c +++ b/plugins/pidfile.c @@ -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) @@ -68,11 +68,11 @@ static void pidfile_update_conds(char *dir, char *name, uint32_t mask) svc_t *svc; paste(fn, sizeof(fn), dir, name); - _d("path: %s, mask: %08x", fn, mask); - if (fnmatch("*\\.pid", fn, 0) && fnmatch("*/pid", fn, 0)) return; + _d("path: %s, mask: %08x", fn, mask); + svc = svc_find_by_pidfile(fn); if (!svc) { _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); 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); if (svc_is_forking(svc)) { pid_t pid; @@ -174,8 +174,6 @@ static void pidfile_callback(void *arg, int fd, int events) if (!ev->mask) continue; - _d("name %s, event: 0x%08x", ev->name, ev->mask); - /* Find base path for this event */ iwp = iwatch_find_by_wd(&iw_pidfile, ev->wd); if (!iwp) @@ -186,12 +184,7 @@ static void pidfile_callback(void *arg, int fd, int events) continue; } - if (ev->mask & IN_DELETE) { - _d("pidfile %s/%s removed ...", iwp->path, ev->name); - continue; - } - - if (ev->mask & (IN_CREATE | IN_ATTRIB | IN_MODIFY | IN_MOVED_TO)) + if (ev->mask & (IN_CLOSE_WRITE | IN_DELETE | IN_ATTRIB | IN_MODIFY | IN_MOVED_TO)) pidfile_update_conds(iwp->path, ev->name, ev->mask); } } diff --git a/src/svc.c b/src/svc.c index b3eb29a0..f6662a05 100644 --- a/src/svc.c +++ b/src/svc.c @@ -415,8 +415,10 @@ svc_t *svc_find_by_pidfile(char *fn) pid_t pid; pid = pid_file_read(fn); - if (pid == -1) + if (pid <= 0) { + _d("pid_file_read(%s) => %d", fn, pid); return NULL; + } for (svc = svc_iterator(&iter, 1); svc; svc = svc_iterator(&iter, 0)) { if (svc->pid != pid)