From e2cc712df40e6d6603f68b9e5f21259c2bb9823d Mon Sep 17 00:00:00 2001
From: ellie timoney <ellie@fastmail.com>
Date: Mon, 5 Jun 2023 10:28:53 +1000
Subject: [PATCH 01/16] proc: decouple proc_settitle from proc_register

---
 backup/backupd.c    |  4 +++-
 imap/fud.c          |  2 +-
 imap/httpd.c        |  5 ++++-
 imap/imapd.c        | 13 ++++++++++---
 imap/nntpd.c        | 12 +++++++++---
 imap/pop3d.c        | 16 +++++++++++++---
 imap/proc.c         | 24 ++++++++++++++++++++++--
 imap/proc.h         | 11 ++++++-----
 imap/setproctitle.c | 12 +++++++++++-
 imap/smmapd.c       |  3 +--
 imap/sync_server.c  |  4 +++-
 ptclient/ptloader.c |  5 ++---
 12 files changed, 85 insertions(+), 26 deletions(-)

diff --git a/backup/backupd.c b/backup/backupd.c
index 04715a561a..a7ea03436a 100644
--- a/backup/backupd.c
+++ b/backup/backupd.c
@@ -184,7 +184,7 @@ EXPORTED int service_init(int argc __attribute__((unused)),
 {
     // FIXME should this be calling fatal? fatal exits directly
     if (geteuid() == 0) fatal("must run as the Cyrus user", EX_USAGE);
-    setproctitle_init(argc, argv, envp);
+    proc_settitle_init(argc, argv, envp);
 
     /* set signal handlers */
     signals_set_shutdown(&shut_down);
@@ -269,6 +269,7 @@ EXPORTED int service_main(int argc __attribute__((unused)),
     }
 
     proc_register(config_ident, backupd_clienthost, NULL, NULL, NULL);
+    proc_settitle(config_ident, backupd_clienthost, NULL, NULL, NULL);
 
     /* Set inactivity timer */
     timeout = config_getduration(IMAPOPT_SYNC_TIMEOUT, 's');
@@ -824,6 +825,7 @@ static void cmd_authenticate(char *mech, char *resp)
 
     backupd_userid = xstrdup((const char *) val);
     proc_register(config_ident, backupd_clienthost, backupd_userid, NULL, NULL);
+    proc_settitle(config_ident, backupd_clienthost, backupd_userid, NULL, NULL);
 
     syslog(LOG_NOTICE, "login: %s %s %s%s %s", backupd_clienthost, backupd_userid,
            mech, backupd_starttls_done ? "+TLS" : "", "User logged in");
diff --git a/imap/fud.c b/imap/fud.c
index e5b1ff32d7..41e559361d 100644
--- a/imap/fud.c
+++ b/imap/fud.c
@@ -157,7 +157,7 @@ int service_init(int argc, char **argv, char **envp)
 {
     if (geteuid() == 0) fatal("must run as the Cyrus user", EX_USAGE);
 
-    setproctitle_init(argc, argv, envp);
+    proc_settitle_init(argc, argv, envp);
 
     signals_set_shutdown(&shut_down);
 
diff --git a/imap/httpd.c b/imap/httpd.c
index 66a2582fbc..d40e600f89 100644
--- a/imap/httpd.c
+++ b/imap/httpd.c
@@ -795,7 +795,7 @@ int service_init(int argc __attribute__((unused)),
     LIBXML_TEST_VERSION
 
     if (geteuid() == 0) fatal("must run as the Cyrus user", EX_USAGE);
-    setproctitle_init(argc, argv, envp);
+    proc_settitle_init(argc, argv, envp);
 
     /* Initialize HTTP connection */
     memset(&http_conn, 0, sizeof(struct http_connection));
@@ -1018,6 +1018,7 @@ int service_main(int argc __attribute__((unused)),
         config_getswitch(IMAPOPT_TLS_REQUIRED) || !avail_auth_schemes;
 
     proc_register(config_ident, http_conn.clienthost, NULL, NULL, NULL);
+    proc_settitle(config_ident, http_conn.clienthost, NULL, NULL, NULL);
 
     /* Construct Alt-Svc header value */
     struct buf buf = BUF_INITIALIZER;
@@ -1935,6 +1936,8 @@ EXPORTED int examine_request(struct transaction_t *txn, const char *uri)
                namespace->prefix);
     proc_register(buf_cstring(&txn->buf), txn->conn->clienthost, httpd_userid,
                   txn->req_tgt.path, txn->req_line.meth);
+    proc_settitle(buf_cstring(&txn->buf), txn->conn->clienthost, httpd_userid,
+                  txn->req_tgt.path, txn->req_line.meth);
     buf_reset(&txn->buf);
 
     /* Request authentication, if necessary */
diff --git a/imap/imapd.c b/imap/imapd.c
index 05a9b444a2..8f1a711941 100644
--- a/imap/imapd.c
+++ b/imap/imapd.c
@@ -984,7 +984,7 @@ int service_init(int argc, char **argv, char **envp)
     int opt, events;
 
     if (geteuid() == 0) fatal("must run as the Cyrus user", EX_USAGE);
-    setproctitle_init(argc, argv, envp);
+    proc_settitle_init(argc, argv, envp);
 
     /* set signal handlers */
     signals_set_shutdown(&shut_down);
@@ -1119,6 +1119,7 @@ int service_main(int argc __attribute__((unused)),
         ((extprops_ssf < 2) && !config_getswitch(IMAPOPT_ALLOWPLAINTEXT));
 
     proc_register(config_ident, imapd_clienthost, NULL, NULL, NULL);
+    proc_settitle(config_ident, imapd_clienthost, NULL, NULL, NULL);
 
     /* Set inactivity timer */
     imapd_timeout = config_getduration(IMAPOPT_TIMEOUT, 'm');
@@ -1428,7 +1429,10 @@ static void cmdloop(void)
         if (backend_current) prot_flush(backend_current->out);
 
         /* command no longer running */
-        proc_register(config_ident, imapd_clienthost, imapd_userid, index_mboxname(imapd_index), NULL);
+        proc_register(config_ident, imapd_clienthost, imapd_userid,
+                      index_mboxname(imapd_index), NULL);
+        proc_settitle(config_ident, imapd_clienthost, imapd_userid,
+                      index_mboxname(imapd_index), NULL);
 
         /* run any delayed cleanup while a user isn't waiting on a reply */
         libcyrus_run_delayed();
@@ -1497,7 +1501,10 @@ static void cmdloop(void)
         if (config_getswitch(IMAPOPT_CHATTY))
             syslog(LOG_NOTICE, "command: %s %s", tag.s, cmd.s);
 
-        proc_register(config_ident, imapd_clienthost, imapd_userid, index_mboxname(imapd_index), cmd.s);
+        proc_register(config_ident, imapd_clienthost, imapd_userid,
+                      index_mboxname(imapd_index), cmd.s);
+        proc_settitle(config_ident, imapd_clienthost, imapd_userid,
+                      index_mboxname(imapd_index), cmd.s);
 
         /* if we need to force a kick, do so */
         if (referral_kick) {
diff --git a/imap/nntpd.c b/imap/nntpd.c
index 17ca4ede18..7877551719 100644
--- a/imap/nntpd.c
+++ b/imap/nntpd.c
@@ -399,7 +399,7 @@ int service_init(int argc __attribute__((unused)),
     initialize_nntp_error_table();
 
     if (geteuid() == 0) fatal("must run as the Cyrus user", EX_USAGE);
-    setproctitle_init(argc, argv, envp);
+    proc_settitle_init(argc, argv, envp);
 
     /* set signal handlers */
     signals_set_shutdown(&shut_down);
@@ -729,7 +729,10 @@ static void cmdloop(void)
 
         signals_poll();
 
-        proc_register(config_ident, nntp_clienthost, nntp_userid, index_mboxname(group_state), NULL);
+        proc_register(config_ident, nntp_clienthost, nntp_userid,
+                      index_mboxname(group_state), NULL);
+        proc_settitle(config_ident, nntp_clienthost, nntp_userid,
+                      index_mboxname(group_state), NULL);
 
         libcyrus_run_delayed();
 
@@ -774,7 +777,10 @@ static void cmdloop(void)
             if (Uisupper(*p)) *p = tolower((unsigned char) *p);
         }
 
-        proc_register(config_ident, nntp_clienthost, nntp_userid, index_mboxname(group_state), cmd.s);
+        proc_register(config_ident, nntp_clienthost, nntp_userid,
+                      index_mboxname(group_state), cmd.s);
+        proc_settitle(config_ident, nntp_clienthost, nntp_userid,
+                      index_mboxname(group_state), cmd.s);
 
         /* Ihave/Takethis only allowed for feeders */
         if (!(nntp_capa & MODE_FEED) &&
diff --git a/imap/pop3d.c b/imap/pop3d.c
index 7e24511f4d..f838be0060 100644
--- a/imap/pop3d.c
+++ b/imap/pop3d.c
@@ -415,7 +415,7 @@ int service_init(int argc __attribute__((unused)),
     int opt;
 
     if (geteuid() == 0) fatal("must run as the Cyrus user", EX_USAGE);
-    setproctitle_init(argc, argv, envp);
+    proc_settitle_init(argc, argv, envp);
 
     /* set signal handlers */
     signals_set_shutdown(&shut_down);
@@ -860,7 +860,12 @@ static void cmdloop(void)
         signals_poll();
 
         /* register process */
-        proc_register(config_ident, popd_clienthost, popd_userid, popd_mailbox ? mailbox_name(popd_mailbox) : NULL, NULL);
+        proc_register(config_ident, popd_clienthost, popd_userid,
+                      popd_mailbox ? mailbox_name(popd_mailbox) : NULL,
+                      NULL);
+        proc_settitle(config_ident, popd_clienthost, popd_userid,
+                      popd_mailbox ? mailbox_name(popd_mailbox) : NULL,
+                      NULL);
 
         libcyrus_run_delayed();
 
@@ -939,7 +944,12 @@ static void cmdloop(void)
             syslog(LOG_NOTICE, "command: %s", inputbuf);
 
         /* register process */
-        proc_register(config_ident, popd_clienthost, popd_userid, popd_mailbox ? mailbox_name(popd_mailbox) : NULL, inputbuf);
+        proc_register(config_ident, popd_clienthost, popd_userid,
+                      popd_mailbox ? mailbox_name(popd_mailbox) : NULL,
+                      inputbuf);
+        proc_settitle(config_ident, popd_clienthost, popd_userid,
+                      popd_mailbox ? mailbox_name(popd_mailbox) : NULL,
+                      inputbuf);
 
         if (!strcmp(inputbuf, "quit")) {
             if (!arg) {
diff --git a/imap/proc.c b/imap/proc.c
index 1961f5d215..51bd18de4c 100644
--- a/imap/proc.c
+++ b/imap/proc.c
@@ -81,6 +81,12 @@
 
 #define FNAME_PROCDIR "/proc"
 
+/* n.b. setproctitle might come from setproctitle.c, or might come from a
+ * system library
+ */
+extern void setproctitle(const char *fmt, ...)
+                         __attribute__((format(printf, 1, 2)));
+
 static char *procfname = 0;
 
 static char *proc_getpath(pid_t pid, int isnew)
@@ -162,8 +168,6 @@ EXPORTED int proc_register(const char *servicename, const char *clienthost,
         fatal("can't write proc file", EX_IOERR);
     }
 
-    setproctitle("%s: %s %s %s %s", servicename, clienthost, userid, mailbox, cmd);
-
     free(newfname);
     return 0;
 }
@@ -409,3 +413,19 @@ EXPORTED void proc_killusercmd(const char *userid, const char *cmd, int sig)
 
     proc_foreach(prockill_cb, &rock);
 }
+
+/* n.b. proc_settitle_init() is defined in setproctitle.c */
+
+EXPORTED void proc_settitle(const char *servicename, const char *clienthost,
+                            const char *userid, const char *mailbox,
+                            const char *cmd)
+{
+    if (!servicename) servicename = "";
+    if (!clienthost) clienthost = "";
+    if (!userid) userid = "";
+    if (!mailbox) mailbox = "";
+    if (!cmd) cmd = "";
+
+    setproctitle("%s: %s %s %s %s",
+                 servicename, clienthost, userid, mailbox, cmd);
+}
diff --git a/imap/proc.h b/imap/proc.h
index a93f7ce4e8..9d089f16ed 100644
--- a/imap/proc.h
+++ b/imap/proc.h
@@ -58,12 +58,9 @@ typedef int procdata_t(pid_t pid,
                        const char *userid, const char *mailbox,
                        const char *cmd, void *rock);
 
-extern void setproctitle_init(int argc, char **argv, char **envp);
-extern void setproctitle(const char *fmt, ...)
-                        __attribute__((format(printf, 1, 2)));
-
 extern int proc_register(const char *servicename, const char *clienthost,
-                         const char *userid, const char *mailbox, const char *cmd);
+                         const char *userid, const char *mailbox,
+                         const char *cmd);
 
 extern void proc_cleanup(void);
 
@@ -75,4 +72,8 @@ extern void proc_killuser(const char *userid);
 extern void proc_killmbox(const char *mboxname);
 extern void proc_killusercmd(const char *userid, const char *cmd, int sig);
 
+extern void proc_settitle_init(int argc, char **argv, char **envp);
+extern void proc_settitle(const char *servicename, const char *clienthost,
+                          const char *userid, const char *mailbox,
+                          const char *cmd);
 #endif /* _PROC_H */
diff --git a/imap/setproctitle.c b/imap/setproctitle.c
index d06c40e0c3..d86c9283d2 100644
--- a/imap/setproctitle.c
+++ b/imap/setproctitle.c
@@ -89,6 +89,12 @@
 
 extern char **environ;
 #ifdef USE_SETPROCTITLE
+/* n.b. USE_SETPROCTITLE is NOT managed by configure!  Thus, this is only ever
+ * defined if the packager has set -DUSE_SETPROCTITLE in CFLAGS or similar
+ * during compile.  So all of this behaviour is usually off, unless your system
+ * provides its own setproctitle(), in which case it'll do whatever it does.
+ * No idea which platforms (if any) provide it...
+ */
 static int setproctitle_enable = 1;
 #else
 static int setproctitle_enable = 0;
@@ -99,8 +105,12 @@ static char             *LastArgv = NULL;       /* end of argv */
 
 /*
  * Sets up a process to be able to use setproctitle()
+ *
+ * This needs to be adjacent to our setproctitle() implementation, if that's
+ * the one being used, which is why it's in here rather than in proc.c with
+ * the rest of the proc_ functions.
  */
-EXPORTED void setproctitle_init(int argc, char **argv, char **envp)
+EXPORTED void proc_settitle_init(int argc, char **argv, char **envp)
 {
     int i;
 
diff --git a/imap/smmapd.c b/imap/smmapd.c
index 740a510789..a54c925f72 100644
--- a/imap/smmapd.c
+++ b/imap/smmapd.c
@@ -175,8 +175,7 @@ int service_init(int argc, char **argv, char **envp)
     int r;
 
     if (geteuid() == 0) fatal("must run as the Cyrus user", EX_USAGE);
-
-    setproctitle_init(argc, argv, envp);
+    proc_settitle_init(argc, argv, envp);
 
     signals_set_shutdown(&shut_down);
     signal(SIGPIPE, SIG_IGN);
diff --git a/imap/sync_server.c b/imap/sync_server.c
index 3a8c9853bf..6d7115f995 100644
--- a/imap/sync_server.c
+++ b/imap/sync_server.c
@@ -242,7 +242,7 @@ int service_init(int argc __attribute__((unused)),
     int opt, r;
 
     if (geteuid() == 0) fatal("must run as the Cyrus user", EX_USAGE);
-    setproctitle_init(argc, argv, envp);
+    proc_settitle_init(argc, argv, envp);
 
     /* set signal handlers */
     signals_set_shutdown(&shut_down_via_signal);
@@ -366,6 +366,7 @@ int service_main(int argc __attribute__((unused)),
     }
 
     proc_register(config_ident, sync_clienthost, NULL, NULL, NULL);
+    proc_settitle(config_ident, sync_clienthost, NULL, NULL, NULL);
 
     /* Set inactivity timer */
     timeout = config_getduration(IMAPOPT_SYNC_TIMEOUT, 's');
@@ -784,6 +785,7 @@ static void cmd_authenticate(char *mech, char *resp)
 
     sync_userid = xstrdup((const char *) val);
     proc_register(config_ident, sync_clienthost, sync_userid, NULL, NULL);
+    proc_settitle(config_ident, sync_clienthost, sync_userid, NULL, NULL);
 
     syslog(LOG_NOTICE, "login: %s %s %s%s %s", sync_clienthost, sync_userid,
            mech, sync_starttls_done ? "+TLS" : "", "User logged in");
diff --git a/ptclient/ptloader.c b/ptclient/ptloader.c
index 388b5cdb61..4ff632a87a 100644
--- a/ptclient/ptloader.c
+++ b/ptclient/ptloader.c
@@ -59,6 +59,7 @@
 #include "auth_pts.h"
 #include "cyrusdb.h"
 #include "imap/global.h"
+#include "imap/proc.h"
 #include "libconfig.h"
 #include "retry.h"
 #include "xmalloc.h"
@@ -74,8 +75,6 @@ struct pts_module *pts_modules[] = {
 #endif
     NULL };
 
-extern void setproctitle_init(int argc, char **argv, char **envp);
-
 static struct pts_module *pts_fromname()
 {
     int i;
@@ -239,7 +238,7 @@ int service_init(int argc, char *argv[], char **envp __attribute__((unused)))
     char *tofree = NULL;
 
     if (geteuid() == 0) fatal("must run as the Cyrus user", EX_USAGE);
-    setproctitle_init(argc, argv, envp);
+    proc_settitle_init(argc, argv, envp);
 
     /* set signal handlers */
     signal(SIGPIPE, SIG_IGN);

From d68e4bef4b4dd9be551c548dac0d1fd248d0d535 Mon Sep 17 00:00:00 2001
From: ellie timoney <ellie@fastmail.com>
Date: Mon, 5 Jun 2023 14:20:50 +1000
Subject: [PATCH 02/16] proc: move from libcyrus_imap to libcyrus_min

will be needed for master to use its functions
---
 Makefile.am                  | 6 +++---
 backup/backupd.c             | 2 +-
 imap/cyr_buildinfo.c         | 2 +-
 {imap => lib}/proc.c         | 2 +-
 {imap => lib}/proc.h         | 0
 {imap => lib}/setproctitle.c | 0
 ptclient/ptloader.c          | 2 +-
 7 files changed, 7 insertions(+), 7 deletions(-)
 rename {imap => lib}/proc.c (99%)
 rename {imap => lib}/proc.h (100%)
 rename {imap => lib}/setproctitle.c (100%)

diff --git a/Makefile.am b/Makefile.am
index decfb2d1f4..76c6d34b1a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -793,6 +793,7 @@ include_HEADERS = \
 	lib/murmurhash2.h \
 	lib/nonblock.h \
 	lib/parseaddr.h \
+	lib/proc.h \
 	lib/procinfo.h \
 	lib/retry.h \
 	lib/rfc822tok.h \
@@ -1065,8 +1066,6 @@ imap_libcyrus_imap_la_SOURCES = \
 	imap/notify.h \
 	imap/partlist.c \
 	imap/partlist.h \
-	imap/proc.c \
-	imap/proc.h \
 	imap/prometheus.c \
 	imap/prometheus.h \
 	imap/protocol.h \
@@ -1087,7 +1086,6 @@ imap_libcyrus_imap_la_SOURCES = \
 	imap/search_sort.h \
 	imap/seen.h \
 	imap/seen_db.c \
-	imap/setproctitle.c \
 	imap/sievedir.c \
 	imap/sievedir.h \
 	imap/smtpclient.c \
@@ -1573,7 +1571,9 @@ lib_libcyrus_min_la_SOURCES = \
 	lib/hashu64.c \
 	lib/libconfig.c \
 	lib/mpool.c \
+	lib/proc.c \
 	lib/retry.c \
+	lib/setproctitle.c \
 	lib/smallarrayu64.c \
 	lib/strarray.c \
 	lib/strhash.c \
diff --git a/backup/backupd.c b/backup/backupd.c
index a7ea03436a..5e9744b3f9 100644
--- a/backup/backupd.c
+++ b/backup/backupd.c
@@ -56,6 +56,7 @@
 #include "lib/bsearch.h"
 #include "lib/imparse.h"
 #include "lib/map.h"
+#include "lib/proc.h"
 #include "lib/signals.h"
 #include "lib/strarray.h"
 #include "lib/util.h"
@@ -63,7 +64,6 @@
 
 #include "imap/global.h"
 #include "imap/imap_err.h"
-#include "imap/proc.h"
 #include "imap/sync_support.h"
 #include "imap/telemetry.h"
 #include "imap/tls.h"
diff --git a/imap/cyr_buildinfo.c b/imap/cyr_buildinfo.c
index 64a35b7c10..9027f42925 100644
--- a/imap/cyr_buildinfo.c
+++ b/imap/cyr_buildinfo.c
@@ -52,13 +52,13 @@
 #include <sysexits.h>
 #include <jansson.h>
 
+#include "lib/proc.h"
 #include "lib/util.h"
 #include "lib/xmalloc.h"
 
 #include "imap/conversations.h"
 #include "imap/global.h"
 #include "imap/mailbox.h"
-#include "imap/proc.h"
 #include "imap/statuscache.h"
 #include "imap/zoneinfo_db.h"
 
diff --git a/imap/proc.c b/lib/proc.c
similarity index 99%
rename from imap/proc.c
rename to lib/proc.c
index 51bd18de4c..d9b720b25c 100644
--- a/imap/proc.c
+++ b/lib/proc.c
@@ -54,7 +54,7 @@
 #include <sys/stat.h>
 
 #include "assert.h"
-#include "global.h"
+#include "libconfig.h"
 #include "proc.h"
 #include "retry.h"
 #include "util.h"
diff --git a/imap/proc.h b/lib/proc.h
similarity index 100%
rename from imap/proc.h
rename to lib/proc.h
diff --git a/imap/setproctitle.c b/lib/setproctitle.c
similarity index 100%
rename from imap/setproctitle.c
rename to lib/setproctitle.c
diff --git a/ptclient/ptloader.c b/ptclient/ptloader.c
index 4ff632a87a..9ebc897a2a 100644
--- a/ptclient/ptloader.c
+++ b/ptclient/ptloader.c
@@ -59,8 +59,8 @@
 #include "auth_pts.h"
 #include "cyrusdb.h"
 #include "imap/global.h"
-#include "imap/proc.h"
 #include "libconfig.h"
+#include "proc.h"
 #include "retry.h"
 #include "xmalloc.h"
 #include "ptloader.h"

From 54d149aca20412d906dfe185d7186ae9b00bbcd3 Mon Sep 17 00:00:00 2001
From: ellie timoney <ellie@fastmail.com>
Date: Wed, 7 Jun 2023 15:40:49 +1000
Subject: [PATCH 03/16] cunit/unit: don't lose log messages when not running
 under valgrind

---
 cunit/unit.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/cunit/unit.c b/cunit/unit.c
index 15af9aa12f..5ab38f7939 100644
--- a/cunit/unit.c
+++ b/cunit/unit.c
@@ -71,15 +71,21 @@ int xml_flag = 0;
 int timeouts_flag = 1;
 
 #if HAVE_VALGRIND_VALGRIND_H
-#define log1(fmt, a1) \
-    VALGRIND_PRINTF_BACKTRACE(fmt"\n", (a1))
-#define log2(fmt, a1, a2) \
-    VALGRIND_PRINTF_BACKTRACE(fmt"\n", (a1), (a2))
+#define log1(fmt, a1) do {                                                  \
+    if (RUNNING_ON_VALGRIND) VALGRIND_PRINTF_BACKTRACE(fmt"\n", (a1));      \
+    else fprintf(stderr, "\nunit: "fmt"\n", (a1));                          \
+} while (0)
+#define log2(fmt, a1, a2) do {                                              \
+    if (RUNNING_ON_VALGRIND) VALGRIND_PRINTF_BACKTRACE(fmt"\n", (a1), (a2));\
+    else fprintf(stderr, "\nunit: "fmt"\n", (a1), (a2));                    \
+} while (0)
 #else
-#define log1(fmt, a1) \
-    fprintf(stderr, "\nunit: "fmt"\n", (a1))
-#define log2(fmt, a1, a2) \
-    fprintf(stderr, "\nunit: "fmt"\n", (a1), (a2))
+#define log1(fmt, a1) do {                                                  \
+    fprintf(stderr, "\nunit: "fmt"\n", (a1));                               \
+} while (0)
+#define log2(fmt, a1, a2) do {                                              \
+    fprintf(stderr, "\nunit: "fmt"\n", (a1), (a2));                         \
+} while (0)
 #endif
 
 jmp_buf fatal_jbuf;

From 59c67408c4073a5f7e8059703126083d5d4b70f4 Mon Sep 17 00:00:00 2001
From: ellie timoney <ellie@fastmail.com>
Date: Wed, 7 Jun 2023 11:49:47 +1000
Subject: [PATCH 04/16] proc.h: put related things together

(this just makes the next commit easier to read)
---
 lib/proc.h | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/lib/proc.h b/lib/proc.h
index 9d089f16ed..152e7b2587 100644
--- a/lib/proc.h
+++ b/lib/proc.h
@@ -43,6 +43,17 @@
 #ifndef _PROC_H
 #define _PROC_H
 
+extern int proc_register(const char *servicename, const char *clienthost,
+                         const char *userid, const char *mailbox,
+                         const char *cmd);
+extern void proc_cleanup(void);
+
+typedef int procdata_t(pid_t pid,
+                       const char *servicename, const char *clienthost,
+                       const char *userid, const char *mailbox,
+                       const char *cmd, void *rock);
+extern int proc_foreach(procdata_t *func, void *rock);
+
 struct proc_limits {
     const char *procname;
     const char *clienthost;
@@ -52,20 +63,6 @@ struct proc_limits {
     int host;
     int maxhost;
 };
-
-typedef int procdata_t(pid_t pid,
-                       const char *servicename, const char *clienthost,
-                       const char *userid, const char *mailbox,
-                       const char *cmd, void *rock);
-
-extern int proc_register(const char *servicename, const char *clienthost,
-                         const char *userid, const char *mailbox,
-                         const char *cmd);
-
-extern void proc_cleanup(void);
-
-extern int proc_foreach(procdata_t *func, void *rock);
-
 extern int proc_checklimits(struct proc_limits *limitsp);
 
 extern void proc_killuser(const char *userid);

From 0e7bdeeafeebfcf735cf4f2ff43a5aac67843a5b Mon Sep 17 00:00:00 2001
From: ellie timoney <ellie@fastmail.com>
Date: Wed, 7 Jun 2023 11:53:40 +1000
Subject: [PATCH 05/16] proc: make proc_register/proc_cleanup re-entrant

with tests!
---
 Makefile.am        |   1 +
 backup/backupd.c   |  11 +-
 cunit/proc.testc   | 394 +++++++++++++++++++++++++++++++++++++++++++++
 imap/http_ws.c     |   3 +-
 imap/httpd.c       |  14 +-
 imap/httpd.h       |   2 +
 imap/imapd.c       |  16 +-
 imap/nntpd.c       |  13 +-
 imap/pop3d.c       |  14 +-
 imap/sync_server.c |  13 +-
 lib/proc.c         |  59 +++++--
 lib/proc.h         |  11 +-
 12 files changed, 502 insertions(+), 49 deletions(-)
 create mode 100644 cunit/proc.testc

diff --git a/Makefile.am b/Makefile.am
index 76c6d34b1a..aa1411098a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -642,6 +642,7 @@ cunit_TESTS = \
 	cunit/msgid.testc \
 	cunit/parseaddr.testc \
 	cunit/parse.testc \
+	cunit/proc.testc \
 	cunit/procinfo.testc \
 	cunit/prot.testc \
 	cunit/ptrarray.testc \
diff --git a/backup/backupd.c b/backup/backupd.c
index 5e9744b3f9..0d0dbf1948 100644
--- a/backup/backupd.c
+++ b/backup/backupd.c
@@ -84,6 +84,7 @@ static sasl_conn_t *backupd_saslconn = NULL;
 static int backupd_starttls_done = 0;
 static int backupd_compress_done = 0;
 static int backupd_logfd = -1;
+static struct proc_handle *proc_handle = NULL;
 
 struct open_backup {
     char *name;
@@ -159,7 +160,7 @@ EXPORTED void fatal(const char* s, int code)
 
     if (recurse_code) {
         /* We were called recursively. Just give up */
-        proc_cleanup();
+        proc_cleanup(&proc_handle);
         exit(recurse_code);
     }
     recurse_code = code;
@@ -268,7 +269,8 @@ EXPORTED int service_main(int argc __attribute__((unused)),
         tcp_disable_nagle(1); /* XXX magic fd */
     }
 
-    proc_register(config_ident, backupd_clienthost, NULL, NULL, NULL);
+    proc_register(&proc_handle, 0,
+                  config_ident, backupd_clienthost, NULL, NULL, NULL);
     proc_settitle(config_ident, backupd_clienthost, NULL, NULL, NULL);
 
     /* Set inactivity timer */
@@ -296,7 +298,7 @@ static void backupd_reset(void)
 {
     open_backups_list_close(&backupd_open_backups, 0);
 
-    proc_cleanup();
+    proc_cleanup(&proc_handle);
 
     if (backupd_in) {
         prot_NONBLOCK(backupd_in);
@@ -824,7 +826,8 @@ static void cmd_authenticate(char *mech, char *resp)
     }
 
     backupd_userid = xstrdup((const char *) val);
-    proc_register(config_ident, backupd_clienthost, backupd_userid, NULL, NULL);
+    proc_register(&proc_handle, 0,
+                  config_ident, backupd_clienthost, backupd_userid, NULL, NULL);
     proc_settitle(config_ident, backupd_clienthost, backupd_userid, NULL, NULL);
 
     syslog(LOG_NOTICE, "login: %s %s %s%s %s", backupd_clienthost, backupd_userid,
diff --git a/cunit/proc.testc b/cunit/proc.testc
new file mode 100644
index 0000000000..091972705c
--- /dev/null
+++ b/cunit/proc.testc
@@ -0,0 +1,394 @@
+#if HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <errno.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include "cunit/cyrunit.h"
+#include "lib/hash.h"
+#include "lib/libconfig.h"
+#include "lib/proc.h"
+#include "lib/util.h"
+
+static char *myconfigdir = NULL;
+
+/* copied declarations: these must match same in lib/proc.c */
+struct proc_handle {
+    pid_t pid;
+    char *fname;
+};
+#define FNAME_PROCDIR "/proc"
+/* end copied declarations */
+
+static int set_up(void)
+{
+    char myconfigdir_template[] = "/tmp/cyrus-cunit-proctestc-XXXXXX";
+    char *dir;
+    struct buf myconfig = BUF_INITIALIZER;
+
+    dir = mkdtemp(myconfigdir_template);
+    if (!dir) return errno;
+
+    myconfigdir = xstrdup(dir);
+    buf_printf(&myconfig, "configdirectory: %s\n", myconfigdir);
+
+    config_read_string(buf_cstring(&myconfig));
+
+    buf_free(&myconfig);
+    return 0;
+}
+
+static int tear_down(void)
+{
+    int r = 0;
+
+    config_reset();
+
+    if (myconfigdir && myconfigdir[0]) {
+        struct buf rm_cmd = BUF_INITIALIZER;
+
+        buf_printf(&rm_cmd, "rm -rf %s", myconfigdir);
+
+        r = system(buf_cstring(&rm_cmd));
+        if (r) r = -1;
+
+        buf_free(&rm_cmd);
+    }
+
+    xzfree(myconfigdir);
+    return r;
+}
+
+static const char *predict_handle_fname(pid_t pid)
+{
+    static char buf[1024];
+
+    memset(buf, 0, sizeof buf);
+    snprintf(buf, sizeof buf, "%s%s/%u", myconfigdir, FNAME_PROCDIR, pid);
+
+    return buf;
+}
+
+static void test_register_self(void)
+{
+    struct proc_handle *handle = NULL;
+    struct proc_handle *savedptr;
+    int r;
+
+    /* first call must create a valid handle */
+    r = proc_register(&handle, 0,
+                      "servicename",
+                      "clienthost",
+                      "userid",
+                      "mailbox",
+                      "cmd");
+    CU_ASSERT_EQUAL(r, 0);
+    CU_ASSERT_PTR_NOT_NULL(handle);
+    CU_ASSERT_NOT_EQUAL(handle->pid, 0);
+    CU_ASSERT_EQUAL(handle->pid, getpid());
+    CU_ASSERT_PTR_NOT_NULL(handle->fname);
+    CU_ASSERT_STRING_EQUAL(handle->fname, predict_handle_fname(getpid()));
+    savedptr = handle;
+
+    /* must be okay to re-register (and keep same handle) */
+    r = proc_register(&handle, 0,
+                      "new_servicename",
+                      "new_clienthost",
+                      "new_userid",
+                      "new_mailbox",
+                      "new_cmd");
+    CU_ASSERT_EQUAL(r, 0);
+    CU_ASSERT_PTR_NOT_NULL(handle);
+    CU_ASSERT_PTR_EQUAL(handle, savedptr);
+    CU_ASSERT_NOT_EQUAL(handle->pid, 0);
+    CU_ASSERT_EQUAL(handle->pid, getpid());
+    CU_ASSERT_PTR_NOT_NULL(handle->fname);
+    CU_ASSERT_STRING_EQUAL(handle->fname, predict_handle_fname(getpid()));
+
+    proc_cleanup(&handle);
+}
+
+static void test_register_other(void)
+{
+    struct proc_handle *handle = NULL;
+    struct proc_handle *savedptr;
+    pid_t pid;
+    int r;
+
+    /* choose some random pid > 0 */
+    do {
+        pid = 1 + rand();
+    } while (pid <= 0 || pid == getpid()); /* whoops, got ours! try again */
+
+    /* first call must create a handle */
+    r = proc_register(&handle, pid,
+                      "servicename",
+                      "clienthost",
+                      "userid",
+                      "mailbox",
+                      "cmd");
+    CU_ASSERT_EQUAL(r, 0);
+    CU_ASSERT_PTR_NOT_NULL(handle);
+    CU_ASSERT_NOT_EQUAL(handle->pid, 0);
+    CU_ASSERT_NOT_EQUAL(handle->pid, getpid());
+    CU_ASSERT_EQUAL(handle->pid, pid);
+    CU_ASSERT_PTR_NOT_NULL(handle->fname);
+    CU_ASSERT_STRING_EQUAL(handle->fname, predict_handle_fname(pid));
+    savedptr = handle;
+
+    /* must be okay to re-register (pid argument must be ignored) */
+    r = proc_register(&handle, 0,
+                      "new_servicename",
+                      "new_clienthost",
+                      "new_userid",
+                      "new_mailbox",
+                      "new_cmd");
+    CU_ASSERT_EQUAL(r, 0);
+    CU_ASSERT_PTR_NOT_NULL(handle);
+    CU_ASSERT_PTR_EQUAL(handle, savedptr);
+    CU_ASSERT_NOT_EQUAL(handle->pid, 0);
+    CU_ASSERT_NOT_EQUAL(handle->pid, getpid());
+    CU_ASSERT_EQUAL(handle->pid, pid);
+    CU_ASSERT_PTR_NOT_NULL(handle->fname);
+    CU_ASSERT_STRING_EQUAL(handle->fname, predict_handle_fname(pid));
+
+    proc_cleanup(&handle);
+}
+
+static void test_cleanup(void)
+{
+    struct proc_handle *handle = NULL;
+    int r;
+
+    /* gotta register something to clean it up... */
+    r = proc_register(&handle, 1,
+                      "servicename",
+                      "clienthost",
+                      "userid",
+                      "mailbox",
+                      "cmd");
+    CU_ASSERT_EQUAL(r, 0);
+    CU_ASSERT_PTR_NOT_NULL(handle);
+    CU_ASSERT_EQUAL(handle->pid, 1);
+    CU_ASSERT_PTR_NOT_NULL(handle->fname);
+    CU_ASSERT_STRING_EQUAL(handle->fname, predict_handle_fname(1));
+
+    /* cleanup had better discard that handle */
+    proc_cleanup(&handle);
+    CU_ASSERT_PTR_NULL(handle);
+
+    /* re-register after cleanup must create a new handle */
+    r = proc_register(&handle, 2,
+                      "servicename",
+                      "clienthost",
+                      "userid",
+                      "mailbox",
+                      "cmd");
+    CU_ASSERT_EQUAL(r, 0);
+    CU_ASSERT_PTR_NOT_NULL(handle);
+    CU_ASSERT_EQUAL(handle->pid, 2);
+    CU_ASSERT_PTR_NOT_NULL(handle->fname);
+    CU_ASSERT_STRING_EQUAL(handle->fname, predict_handle_fname(2));
+
+    /* cleanup had better discard that one too */
+    proc_cleanup(&handle);
+    CU_ASSERT_PTR_NULL(handle);
+}
+
+struct procdata_fields {
+    char *servicename;
+    char *clienthost;
+    char *userid;
+    char *mailbox;
+    char *cmd;
+};
+
+static void free_procdata_fields(void *p)
+{
+    struct procdata_fields *f = (struct procdata_fields *) p;
+
+    free(f->servicename);
+    free(f->clienthost);
+    free(f->userid);
+    free(f->mailbox);
+    free(f->cmd);
+    free(f);
+}
+
+static int collect_procs_cb(pid_t pid,
+                            const char *servicename,
+                            const char *clienthost,
+                            const char *userid,
+                            const char *mailbox,
+                            const char *cmd,
+                            void *rock)
+{
+    char pid_str[32] = {0};
+    hash_table *results = (hash_table *) rock;
+
+    snprintf(pid_str, sizeof pid_str, "%u", pid);
+
+    struct procdata_fields *fields = xmalloc(sizeof *fields);
+    fields->servicename = xstrdupnull(servicename);
+    fields->clienthost = xstrdupnull(clienthost);
+    fields->userid = xstrdupnull(userid);
+    fields->mailbox = xstrdupnull(mailbox);
+    fields->cmd = xstrdupnull(cmd);
+
+    /* better not have seen this pid already! */
+    CU_ASSERT_PTR_NULL(hash_lookup(pid_str, results));
+
+    hash_insert(pid_str, fields, results);
+    return 0;
+}
+
+static void test_proc_foreach(void)
+{
+    struct {
+        struct proc_handle *handle;
+        struct procdata_fields fields;
+    } tests[] = {
+        { NULL, { "sn0", "ch0", "ui0", "mb0", "cm0" } },
+        { NULL, { "sn1", "ch1", "ui1", "mb1", "cm1" } },
+        { NULL, { "sn2", "ch2", "ui2", "mb2", "cm2" } },
+        { NULL, { "sn3", "ch3", "ui3", "mb3", "cm3" } },
+        { NULL, { "sn4", "ch4", "ui4", "mb4", "cm4" } },
+        { NULL, { "sn5", "ch5", "ui5", "mb5", "cm5" } },
+    };
+    const size_t n_tests = sizeof(tests) / sizeof(tests[0]);
+    const pid_t mypid = getpid();
+    hash_table results = HASH_TABLE_INITIALIZER;
+    strarray_t *keys = NULL;
+    int i, r;
+
+    /* register our "processes" */
+    for (i = 0; (unsigned) i < n_tests; i++) {
+        r = proc_register(&tests[i].handle,
+                          i, /* use test index as pid */
+                          tests[i].fields.servicename,
+                          tests[i].fields.clienthost,
+                          tests[i].fields.userid,
+                          tests[i].fields.mailbox,
+                          tests[i].fields.cmd);
+        CU_ASSERT_EQUAL(r, 0);
+        CU_ASSERT_PTR_NOT_NULL(&tests[i].handle);
+        if (i == 0) {
+            CU_ASSERT_EQUAL(tests[i].handle->pid, mypid);
+        }
+        else {
+            CU_ASSERT_EQUAL(tests[i].handle->pid, i);
+        }
+    }
+
+    /* let's see if it finds everything */
+    construct_hash_table(&results, n_tests, 0);
+    r = proc_foreach(&collect_procs_cb, &results);
+    CU_ASSERT_EQUAL(r, 0);
+    for (i = 0; (unsigned) i < n_tests; i++) {
+        char pid_str[32] = {0};
+        struct procdata_fields *fields;
+
+        snprintf(pid_str, sizeof pid_str, "%d", i ? i : mypid);
+
+        fields = hash_lookup(pid_str, &results);
+        CU_ASSERT_PTR_NOT_NULL(fields);
+        CU_ASSERT_STRING_EQUAL(fields->servicename, tests[i].fields.servicename);
+        CU_ASSERT_STRING_EQUAL(fields->clienthost, tests[i].fields.clienthost);
+        CU_ASSERT_STRING_EQUAL(fields->userid, tests[i].fields.userid);
+        CU_ASSERT_STRING_EQUAL(fields->mailbox, tests[i].fields.mailbox);
+        CU_ASSERT_STRING_EQUAL(fields->cmd, tests[i].fields.cmd);
+    }
+
+    /* better not have found anything extra! */
+    keys = hash_keys(&results);
+    for (i = 0; i < strarray_size(keys); i++) {
+        int found_pid = atoi(strarray_nth(keys, i));
+
+        /* real process pid will be out of range but is legit */
+        if (found_pid == mypid) continue;
+
+        /* better not see a pid 0! */
+        CU_ASSERT(found_pid > 0);
+
+        /* better not see anything higher than those we created */
+        CU_ASSERT((unsigned) found_pid < n_tests);
+    }
+
+    /* reset results */
+    free_hash_table(&results, &free_procdata_fields);
+
+    /* reregistering with different strings should work */
+    for (i = 0; (unsigned) i < n_tests; i++) {
+        r = proc_register(&tests[i].handle,
+                          i, /* use test index as pid */
+                          "new servicename",
+                          "new clienthost",
+                          "new userid",
+                          "new mailbox",
+                          "new cmd");
+        CU_ASSERT_EQUAL(r, 0);
+        CU_ASSERT_PTR_NOT_NULL(&tests[i].handle);
+        if (i == 0) {
+            CU_ASSERT_EQUAL(tests[i].handle->pid, mypid);
+        }
+        else {
+            CU_ASSERT_EQUAL(tests[i].handle->pid, i);
+        }
+    }
+
+    /* let's see if it finds everything */
+    construct_hash_table(&results, n_tests, 0);
+    r = proc_foreach(&collect_procs_cb, &results);
+    CU_ASSERT_EQUAL(r, 0);
+    for (i = 0; (unsigned) i < n_tests; i++) {
+        char pid_str[32] = {0};
+        struct procdata_fields *fields;
+
+        snprintf(pid_str, sizeof pid_str, "%u", i ? i : mypid);
+
+        fields = hash_lookup(pid_str, &results);
+        CU_ASSERT_PTR_NOT_NULL(fields);
+        CU_ASSERT_STRING_EQUAL(fields->servicename, "new servicename");
+        CU_ASSERT_STRING_EQUAL(fields->clienthost, "new clienthost");
+        CU_ASSERT_STRING_EQUAL(fields->userid, "new userid");
+        CU_ASSERT_STRING_EQUAL(fields->mailbox, "new mailbox");
+        CU_ASSERT_STRING_EQUAL(fields->cmd, "new cmd");
+    }
+
+    /* better not have found anything extra! */
+    keys = hash_keys(&results);
+    for (i = 0; i < strarray_size(keys); i++) {
+        int found_pid = atoi(strarray_nth(keys, i));
+
+        /* real process pid will be out of range but is legit */
+        if (found_pid == mypid) continue;
+
+        /* better not see a pid 0! */
+        CU_ASSERT(found_pid > 0);
+
+        /* better not see anything higher than those we created */
+        CU_ASSERT((unsigned) found_pid < n_tests);
+    }
+
+    /* reset results */
+    free_hash_table(&results, &free_procdata_fields);
+
+    /* cleanup our "processes" */
+    for (i = 0; (unsigned ) i < n_tests; i++) {
+        proc_cleanup(&tests[i].handle);
+        CU_ASSERT_PTR_NULL(tests[i].handle);
+    }
+
+    /* shouldn't find anything this time */
+    construct_hash_table(&results, n_tests, 0);
+    r = proc_foreach(&collect_procs_cb, &results);
+    CU_ASSERT_EQUAL(r, 0);
+    CU_ASSERT_EQUAL(hash_numrecords(&results), 0);
+
+    /* and we're finished */
+    free_hash_table(&results, &free_procdata_fields);
+}
+
+/* vim: set ft=c: */
diff --git a/imap/http_ws.c b/imap/http_ws.c
index a0f3604ca9..a00102eb3e 100644
--- a/imap/http_ws.c
+++ b/imap/http_ws.c
@@ -898,7 +898,8 @@ HIDDEN int ws_start_channel(struct transaction_t *txn,
     buf_printf(&service, "%s%s", config_ident,
                namespace->well_known ? strrchr(namespace->well_known, '/') :
                namespace->prefix);
-    proc_register(buf_cstring(&service), txn->conn->clienthost, httpd_userid,
+    proc_register(&httpd_proc_handle, 0,
+                  buf_cstring(&service), txn->conn->clienthost, httpd_userid,
                   txn->req_tgt.path, "WS");
     buf_free(&service);
 
diff --git a/imap/httpd.c b/imap/httpd.c
index d40e600f89..0f71a634de 100644
--- a/imap/httpd.c
+++ b/imap/httpd.c
@@ -114,6 +114,8 @@
 
 static unsigned accept_encodings = 0;
 
+struct proc_handle *httpd_proc_handle = NULL;
+
 #ifdef HAVE_ZLIB
 #include <zlib.h>
 
@@ -694,7 +696,7 @@ static void httpd_reset(struct http_connection *conn)
     /* Reset available authentication schemes */
     avail_auth_schemes = 0;
 
-    proc_cleanup();
+    proc_cleanup(&httpd_proc_handle);
 
     /* close backend connections */
     for (i = 0; i < ptrarray_size(&backend_cached); i++) {
@@ -1017,7 +1019,8 @@ int service_main(int argc __attribute__((unused)),
     httpd_tls_required =
         config_getswitch(IMAPOPT_TLS_REQUIRED) || !avail_auth_schemes;
 
-    proc_register(config_ident, http_conn.clienthost, NULL, NULL, NULL);
+    proc_register(&httpd_proc_handle, 0,
+                  config_ident, http_conn.clienthost, NULL, NULL, NULL);
     proc_settitle(config_ident, http_conn.clienthost, NULL, NULL, NULL);
 
     /* Construct Alt-Svc header value */
@@ -1133,7 +1136,7 @@ void shut_down(int code)
 
     xmlCleanupParser();
 
-    proc_cleanup();
+    proc_cleanup(&httpd_proc_handle);
 
     /* close backend connections */
     for (i = 0; i < ptrarray_size(&backend_cached); i++) {
@@ -1191,7 +1194,7 @@ EXPORTED void fatal(const char* s, int code)
 
     if (recurse_code) {
         /* We were called recursively. Just give up */
-        proc_cleanup();
+        proc_cleanup(&httpd_proc_handle);
         if (httpd_out) {
             /* one less active connection */
             prometheus_decrement(CYRUS_HTTP_ACTIVE_CONNECTIONS);
@@ -1934,7 +1937,8 @@ EXPORTED int examine_request(struct transaction_t *txn, const char *uri)
     buf_printf(&txn->buf, "%s%s", config_ident,
                namespace->well_known ? strrchr(namespace->well_known, '/') :
                namespace->prefix);
-    proc_register(buf_cstring(&txn->buf), txn->conn->clienthost, httpd_userid,
+    proc_register(&httpd_proc_handle, 0,
+                  buf_cstring(&txn->buf), txn->conn->clienthost, httpd_userid,
                   txn->req_tgt.path, txn->req_line.meth);
     proc_settitle(buf_cstring(&txn->buf), txn->conn->clienthost, httpd_userid,
                   txn->req_tgt.path, txn->req_line.meth);
diff --git a/imap/httpd.h b/imap/httpd.h
index 6583ece5c7..3781fcaa9f 100644
--- a/imap/httpd.h
+++ b/imap/httpd.h
@@ -585,6 +585,8 @@ extern int ignorequota;
 extern int apns_enabled;
 extern int ws_enabled;
 
+extern struct proc_handle *httpd_proc_handle;
+
 extern xmlURIPtr parse_uri(unsigned meth, const char *uri, unsigned path_reqd,
                            const char **errstr);
 extern dynarray_t *parse_accept(const char **hdr);
diff --git a/imap/imapd.c b/imap/imapd.c
index 8f1a711941..b08d96b01b 100644
--- a/imap/imapd.c
+++ b/imap/imapd.c
@@ -131,6 +131,7 @@ extern char *optarg;
 
 /* global state */
 const int config_need_data = CONFIG_NEED_PARTITION_DATA;
+static struct proc_handle *proc_handle = NULL;
 
 static int imaps = 0;
 static struct saslprops_t saslprops = SASLPROPS_INITIALIZER;
@@ -866,7 +867,7 @@ static void imapd_reset(void)
     /* run delayed commands first before closing anything */
     libcyrus_run_delayed();
 
-    proc_cleanup();
+    proc_cleanup(&proc_handle);
 
     /* close backend connections */
     for (i = 0; i < ptrarray_size(&backend_cached); i++) {
@@ -1118,7 +1119,8 @@ int service_main(int argc __attribute__((unused)),
     imapd_login_disabled = imapd_tls_required ||
         ((extprops_ssf < 2) && !config_getswitch(IMAPOPT_ALLOWPLAINTEXT));
 
-    proc_register(config_ident, imapd_clienthost, NULL, NULL, NULL);
+    proc_register(&proc_handle, 0,
+                  config_ident, imapd_clienthost, NULL, NULL, NULL);
     proc_settitle(config_ident, imapd_clienthost, NULL, NULL, NULL);
 
     /* Set inactivity timer */
@@ -1228,7 +1230,7 @@ void shut_down(int code)
     /* run delayed commands before we take away all the environment */
     libcyrus_run_delayed();
 
-    proc_cleanup();
+    proc_cleanup(&proc_handle);
 
     for (i = 0; i < ptrarray_size(&backend_cached); i++) {
         struct backend *be = ptrarray_nth(&backend_cached, i);
@@ -1312,7 +1314,7 @@ EXPORTED void fatal(const char *s, int code)
 
     if (recurse_code) {
         /* We were called recursively. Just give up */
-        proc_cleanup();
+        proc_cleanup(&proc_handle);
         if (imapd_out) {
             /* one less active connection */
             prometheus_decrement(CYRUS_IMAP_ACTIVE_CONNECTIONS);
@@ -1429,7 +1431,8 @@ static void cmdloop(void)
         if (backend_current) prot_flush(backend_current->out);
 
         /* command no longer running */
-        proc_register(config_ident, imapd_clienthost, imapd_userid,
+        proc_register(&proc_handle, 0,
+                      config_ident, imapd_clienthost, imapd_userid,
                       index_mboxname(imapd_index), NULL);
         proc_settitle(config_ident, imapd_clienthost, imapd_userid,
                       index_mboxname(imapd_index), NULL);
@@ -1501,7 +1504,8 @@ static void cmdloop(void)
         if (config_getswitch(IMAPOPT_CHATTY))
             syslog(LOG_NOTICE, "command: %s %s", tag.s, cmd.s);
 
-        proc_register(config_ident, imapd_clienthost, imapd_userid,
+        proc_register(&proc_handle, 0,
+                      config_ident, imapd_clienthost, imapd_userid,
                       index_mboxname(imapd_index), cmd.s);
         proc_settitle(config_ident, imapd_clienthost, imapd_userid,
                       index_mboxname(imapd_index), cmd.s);
diff --git a/imap/nntpd.c b/imap/nntpd.c
index 7877551719..f5140d96e5 100644
--- a/imap/nntpd.c
+++ b/imap/nntpd.c
@@ -151,6 +151,7 @@ static int allowanonymous = 0;
 static int singleinstance = 1;  /* attempt single instance store */
 
 static struct stagemsg *stage = NULL;
+static struct proc_handle *proc_handle = NULL;
 
 /* Bitmasks for NNTP modes */
 enum {
@@ -316,7 +317,7 @@ static void nntp_reset(void)
 {
     int i;
 
-    proc_cleanup();
+    proc_cleanup(&proc_handle);
 
     /* close local mailbox */
     if (group_state)
@@ -574,7 +575,7 @@ void shut_down(int code)
 
     libcyrus_run_delayed();
 
-    proc_cleanup();
+    proc_cleanup(&proc_handle);
 
     /* close local mailbox */
     if (group_state)
@@ -628,7 +629,7 @@ EXPORTED void fatal(const char* s, int code)
 
     if (recurse_code) {
         /* We were called recursively. Just give up */
-        proc_cleanup();
+        proc_cleanup(&proc_handle);
         exit(recurse_code);
     }
     recurse_code = code;
@@ -729,7 +730,8 @@ static void cmdloop(void)
 
         signals_poll();
 
-        proc_register(config_ident, nntp_clienthost, nntp_userid,
+        proc_register(&proc_handle, 0,
+                      config_ident, nntp_clienthost, nntp_userid,
                       index_mboxname(group_state), NULL);
         proc_settitle(config_ident, nntp_clienthost, nntp_userid,
                       index_mboxname(group_state), NULL);
@@ -777,7 +779,8 @@ static void cmdloop(void)
             if (Uisupper(*p)) *p = tolower((unsigned char) *p);
         }
 
-        proc_register(config_ident, nntp_clienthost, nntp_userid,
+        proc_register(&proc_handle, 0,
+                      config_ident, nntp_clienthost, nntp_userid,
                       index_mboxname(group_state), cmd.s);
         proc_settitle(config_ident, nntp_clienthost, nntp_userid,
                       index_mboxname(group_state), cmd.s);
diff --git a/imap/pop3d.c b/imap/pop3d.c
index f838be0060..7127755c5f 100644
--- a/imap/pop3d.c
+++ b/imap/pop3d.c
@@ -156,6 +156,8 @@ static int popd_tls_required = 0;
 
 static int popd_myrights;
 
+static struct proc_handle *proc_handle = NULL;
+
 /* the sasl proxy policy context */
 static struct proxy_context popd_proxyctx = {
     0, 1, &popd_authstate, NULL, NULL
@@ -328,7 +330,7 @@ static void popd_reset(void)
     int bytes_in = 0;
     int bytes_out = 0;
 
-    proc_cleanup();
+    proc_cleanup(&proc_handle);
 
     syslog(LOG_NOTICE, "counts: retr=<%d> top=<%d> dele=<%d>",
                        count_retr, count_top, count_dele);
@@ -605,7 +607,7 @@ void shut_down(int code)
 
     libcyrus_run_delayed();
 
-    proc_cleanup();
+    proc_cleanup(&proc_handle);
 
     /* close local mailbox */
     if (popd_mailbox)
@@ -676,7 +678,7 @@ EXPORTED void fatal(const char* s, int code)
 
     if (recurse_code) {
         /* We were called recursively. Just give up */
-        proc_cleanup();
+        proc_cleanup(&proc_handle);
         exit(recurse_code);
     }
     recurse_code = code;
@@ -860,7 +862,8 @@ static void cmdloop(void)
         signals_poll();
 
         /* register process */
-        proc_register(config_ident, popd_clienthost, popd_userid,
+        proc_register(&proc_handle, 0,
+                      config_ident, popd_clienthost, popd_userid,
                       popd_mailbox ? mailbox_name(popd_mailbox) : NULL,
                       NULL);
         proc_settitle(config_ident, popd_clienthost, popd_userid,
@@ -944,7 +947,8 @@ static void cmdloop(void)
             syslog(LOG_NOTICE, "command: %s", inputbuf);
 
         /* register process */
-        proc_register(config_ident, popd_clienthost, popd_userid,
+        proc_register(&proc_handle, 0,
+                      config_ident, popd_clienthost, popd_userid,
                       popd_mailbox ? mailbox_name(popd_mailbox) : NULL,
                       inputbuf);
         proc_settitle(config_ident, popd_clienthost, popd_userid,
diff --git a/imap/sync_server.c b/imap/sync_server.c
index 6d7115f995..7e403c5fc9 100644
--- a/imap/sync_server.c
+++ b/imap/sync_server.c
@@ -136,6 +136,7 @@ static int sync_starttls_done = 0;
 static int sync_compress_done = 0;
 static int sync_sieve_mailbox_enabled = 0;
 static int sync_archive_enabled = 0;
+static struct proc_handle *proc_handle = NULL;
 
 static int opt_force = 0;
 
@@ -180,7 +181,7 @@ static struct sasl_callback mysasl_cb[] = {
 
 static void sync_reset(void)
 {
-    proc_cleanup();
+    proc_cleanup(&proc_handle);
 
     if (sync_in) {
         prot_NONBLOCK(sync_in);
@@ -365,7 +366,8 @@ int service_main(int argc __attribute__((unused)),
         tcp_disable_nagle(1); /* XXX magic fd */
     }
 
-    proc_register(config_ident, sync_clienthost, NULL, NULL, NULL);
+    proc_register(&proc_handle, 0,
+                  config_ident, sync_clienthost, NULL, NULL, NULL);
     proc_settitle(config_ident, sync_clienthost, NULL, NULL, NULL);
 
     /* Set inactivity timer */
@@ -413,7 +415,7 @@ void shut_down(int code)
 
     libcyrus_run_delayed();
 
-    proc_cleanup();
+    proc_cleanup(&proc_handle);
 
     seen_done();
 
@@ -456,7 +458,7 @@ EXPORTED void fatal(const char* s, int code)
 
     if (recurse_code) {
         /* We were called recursively. Just give up */
-        proc_cleanup();
+        proc_cleanup(&proc_handle);
         exit(recurse_code);
     }
     recurse_code = code;
@@ -784,7 +786,8 @@ static void cmd_authenticate(char *mech, char *resp)
     }
 
     sync_userid = xstrdup((const char *) val);
-    proc_register(config_ident, sync_clienthost, sync_userid, NULL, NULL);
+    proc_register(&proc_handle, 0,
+                  config_ident, sync_clienthost, sync_userid, NULL, NULL);
     proc_settitle(config_ident, sync_clienthost, sync_userid, NULL, NULL);
 
     syslog(LOG_NOTICE, "login: %s %s %s%s %s", sync_clienthost, sync_userid,
diff --git a/lib/proc.c b/lib/proc.c
index d9b720b25c..f13c53cf27 100644
--- a/lib/proc.c
+++ b/lib/proc.c
@@ -87,8 +87,6 @@
 extern void setproctitle(const char *fmt, ...)
                          __attribute__((format(printf, 1, 2)));
 
-static char *procfname = 0;
-
 static char *proc_getpath(pid_t pid, int isnew)
 {
     struct buf buf = BUF_INITIALIZER;
@@ -127,15 +125,36 @@ static char *proc_getdir(void)
     return proc_getpath(0, 0);
 }
 
-EXPORTED int proc_register(const char *servicename, const char *clienthost,
-                  const char *userid, const char *mailbox, const char *cmd)
+struct proc_handle {
+    pid_t pid;
+    char *fname;
+};
+
+EXPORTED int proc_register(struct proc_handle **handlep,
+                           pid_t pid,
+                           const char *servicename,
+                           const char *clienthost,
+                           const char *userid,
+                           const char *mailbox,
+                           const char *cmd)
 {
-    pid_t pid = getpid();
     FILE *procfile = NULL;
     char *newfname = NULL;
+    struct proc_handle *handle = NULL;
 
-    if (!procfname)
-        procfname = proc_getpath(pid, /*isnew*/0);
+    assert(handlep != NULL);
+
+    if (*handlep != NULL) {
+        handle = *handlep;
+        pid = handle->pid;
+    }
+    else {
+        handle = xmalloc(sizeof *handle);
+        if (!pid) pid = getpid();
+        handle->pid = pid;
+        handle->fname = proc_getpath(pid, /*isnew*/0);
+        *handlep = handle;
+    }
 
     newfname = proc_getpath(pid, /*isnew*/1);
 
@@ -159,11 +178,14 @@ EXPORTED int proc_register(const char *servicename, const char *clienthost,
     if (!userid) userid = "";
     if (!mailbox) mailbox = "";
     if (!cmd) cmd = "";
-    fprintf(procfile, "%s\t%s\t%s\t%s\t%s\n", servicename, clienthost, userid, mailbox, cmd);
+    fprintf(procfile, "%s\t%s\t%s\t%s\t%s\n",
+                      servicename, clienthost, userid, mailbox, cmd);
     fclose(procfile);
 
-    if (rename(newfname, procfname)) {
-        syslog(LOG_ERR, "IOERROR: renaming %s to %s: %m", newfname, procfname);
+    if (rename(newfname, handle->fname)) {
+        xsyslog(LOG_ERR, "IOERROR: rename failed",
+                         "source=<%s> dest=<%s>",
+                         newfname, handle->fname);
         xunlink(newfname);
         fatal("can't write proc file", EX_IOERR);
     }
@@ -172,12 +194,19 @@ EXPORTED int proc_register(const char *servicename, const char *clienthost,
     return 0;
 }
 
-EXPORTED void proc_cleanup(void)
+EXPORTED void proc_cleanup(struct proc_handle **handlep)
 {
-    if (procfname) {
-        xunlink(procfname);
-        free(procfname);
-        procfname = NULL;
+    struct proc_handle *handle;
+
+    assert(handlep != NULL);
+
+    handle = *handlep;
+    *handlep = NULL;
+
+    if (handle) {
+        xunlink(handle->fname);
+        free(handle->fname);
+        free(handle);
     }
 }
 
diff --git a/lib/proc.h b/lib/proc.h
index 152e7b2587..cb45f9fb0a 100644
--- a/lib/proc.h
+++ b/lib/proc.h
@@ -43,10 +43,15 @@
 #ifndef _PROC_H
 #define _PROC_H
 
-extern int proc_register(const char *servicename, const char *clienthost,
-                         const char *userid, const char *mailbox,
+struct proc_handle;
+extern int proc_register(struct proc_handle **handlep,
+                         pid_t pid,
+                         const char *servicename,
+                         const char *clienthost,
+                         const char *userid,
+                         const char *mailbox,
                          const char *cmd);
-extern void proc_cleanup(void);
+extern void proc_cleanup(struct proc_handle **handlep);
 
 typedef int procdata_t(pid_t pid,
                        const char *servicename, const char *clienthost,

From f78658ce252a66837f4e2dab99cb82cea1cd9898 Mon Sep 17 00:00:00 2001
From: ellie timoney <ellie@fastmail.com>
Date: Fri, 9 Jun 2023 13:33:59 +1000
Subject: [PATCH 06/16] proc: make proc_register errors non-fatal

and existing callers now check the return value and fatal themselves
---
 backup/backupd.c   | 13 ++++++++-----
 imap/http_ws.c     |  7 ++++---
 imap/httpd.c       | 15 +++++++++------
 imap/imapd.c       | 22 ++++++++++++++--------
 imap/nntpd.c       | 14 ++++++++------
 imap/pop3d.c       | 19 +++++++++++--------
 imap/sync_server.c | 12 +++++++-----
 lib/proc.c         | 23 +++++++++++++++++++----
 8 files changed, 80 insertions(+), 45 deletions(-)

diff --git a/backup/backupd.c b/backup/backupd.c
index 0d0dbf1948..fa93242d8d 100644
--- a/backup/backupd.c
+++ b/backup/backupd.c
@@ -223,7 +223,7 @@ EXPORTED int service_main(int argc __attribute__((unused)),
 {
     const char *localip, *remoteip;
     sasl_security_properties_t *secprops = NULL;
-    int timeout;
+    int r, timeout;
 
     signals_poll();
 
@@ -269,8 +269,9 @@ EXPORTED int service_main(int argc __attribute__((unused)),
         tcp_disable_nagle(1); /* XXX magic fd */
     }
 
-    proc_register(&proc_handle, 0,
-                  config_ident, backupd_clienthost, NULL, NULL, NULL);
+    r = proc_register(&proc_handle, 0,
+                      config_ident, backupd_clienthost, NULL, NULL, NULL);
+    if (r) fatal("unable to register process", EX_IOERR);
     proc_settitle(config_ident, backupd_clienthost, NULL, NULL, NULL);
 
     /* Set inactivity timer */
@@ -826,8 +827,10 @@ static void cmd_authenticate(char *mech, char *resp)
     }
 
     backupd_userid = xstrdup((const char *) val);
-    proc_register(&proc_handle, 0,
-                  config_ident, backupd_clienthost, backupd_userid, NULL, NULL);
+    r = proc_register(&proc_handle, 0,
+                      config_ident, backupd_clienthost, backupd_userid,
+                      NULL, NULL);
+    if (r) fatal("unable to register process", EX_IOERR);
     proc_settitle(config_ident, backupd_clienthost, backupd_userid, NULL, NULL);
 
     syslog(LOG_NOTICE, "login: %s %s %s%s %s", backupd_clienthost, backupd_userid,
diff --git a/imap/http_ws.c b/imap/http_ws.c
index a00102eb3e..c74fe5e232 100644
--- a/imap/http_ws.c
+++ b/imap/http_ws.c
@@ -898,9 +898,10 @@ HIDDEN int ws_start_channel(struct transaction_t *txn,
     buf_printf(&service, "%s%s", config_ident,
                namespace->well_known ? strrchr(namespace->well_known, '/') :
                namespace->prefix);
-    proc_register(&httpd_proc_handle, 0,
-                  buf_cstring(&service), txn->conn->clienthost, httpd_userid,
-                  txn->req_tgt.path, "WS");
+    r = proc_register(&httpd_proc_handle, 0,
+                      buf_cstring(&service), txn->conn->clienthost,
+                      httpd_userid, txn->req_tgt.path, "WS");
+    if (r) fatal("unable to register process", EX_IOERR);
     buf_free(&service);
 
     return 0;
diff --git a/imap/httpd.c b/imap/httpd.c
index 0f71a634de..8674376359 100644
--- a/imap/httpd.c
+++ b/imap/httpd.c
@@ -935,6 +935,7 @@ int service_main(int argc __attribute__((unused)),
     int mechcount = 0;
     size_t mechlen;
     struct auth_scheme_t *scheme;
+    int r;
 
     /* fatal/shut_down will adjust these, so we need to set them early */
     prometheus_decrement(CYRUS_HTTP_READY_LISTENERS);
@@ -1019,8 +1020,9 @@ int service_main(int argc __attribute__((unused)),
     httpd_tls_required =
         config_getswitch(IMAPOPT_TLS_REQUIRED) || !avail_auth_schemes;
 
-    proc_register(&httpd_proc_handle, 0,
-                  config_ident, http_conn.clienthost, NULL, NULL, NULL);
+    r = proc_register(&httpd_proc_handle, 0,
+                      config_ident, http_conn.clienthost, NULL, NULL, NULL);
+    if (r) fatal("unable to register process", EX_IOERR);
     proc_settitle(config_ident, http_conn.clienthost, NULL, NULL, NULL);
 
     /* Construct Alt-Svc header value */
@@ -1906,7 +1908,7 @@ static void postauth_check_hdrs(struct transaction_t *txn)
 
 EXPORTED int examine_request(struct transaction_t *txn, const char *uri)
 {
-    int ret = 0, sasl_result = 0;
+    int r, ret = 0, sasl_result = 0;
     const char *query;
     const struct namespace_t *namespace;
     struct request_line_t *req_line = &txn->req_line;
@@ -1937,9 +1939,10 @@ EXPORTED int examine_request(struct transaction_t *txn, const char *uri)
     buf_printf(&txn->buf, "%s%s", config_ident,
                namespace->well_known ? strrchr(namespace->well_known, '/') :
                namespace->prefix);
-    proc_register(&httpd_proc_handle, 0,
-                  buf_cstring(&txn->buf), txn->conn->clienthost, httpd_userid,
-                  txn->req_tgt.path, txn->req_line.meth);
+    r = proc_register(&httpd_proc_handle, 0,
+                      buf_cstring(&txn->buf), txn->conn->clienthost,
+                      httpd_userid, txn->req_tgt.path, txn->req_line.meth);
+    if (r) fatal("unable to register process", EX_IOERR);
     proc_settitle(buf_cstring(&txn->buf), txn->conn->clienthost, httpd_userid,
                   txn->req_tgt.path, txn->req_line.meth);
     buf_reset(&txn->buf);
diff --git a/imap/imapd.c b/imap/imapd.c
index b08d96b01b..35d84ab1a0 100644
--- a/imap/imapd.c
+++ b/imap/imapd.c
@@ -1069,6 +1069,7 @@ int service_main(int argc __attribute__((unused)),
     struct mboxevent *mboxevent = NULL;
     struct io_count *io_count_start = NULL;
     struct io_count *io_count_stop = NULL;
+    int r;
 
     /* fatal/shut_down will adjust these, so we need to set them early */
     prometheus_decrement(CYRUS_IMAP_READY_LISTENERS);
@@ -1119,8 +1120,9 @@ int service_main(int argc __attribute__((unused)),
     imapd_login_disabled = imapd_tls_required ||
         ((extprops_ssf < 2) && !config_getswitch(IMAPOPT_ALLOWPLAINTEXT));
 
-    proc_register(&proc_handle, 0,
-                  config_ident, imapd_clienthost, NULL, NULL, NULL);
+    r = proc_register(&proc_handle, 0,
+                      config_ident, imapd_clienthost, NULL, NULL, NULL);
+    if (r) fatal("unable to register process", EX_IOERR);
     proc_settitle(config_ident, imapd_clienthost, NULL, NULL, NULL);
 
     /* Set inactivity timer */
@@ -1417,6 +1419,8 @@ static void cmdloop(void)
     }
 
     for (;;) {
+        int r;
+
         /* Release any held index */
         index_release(imapd_index);
 
@@ -1431,9 +1435,10 @@ static void cmdloop(void)
         if (backend_current) prot_flush(backend_current->out);
 
         /* command no longer running */
-        proc_register(&proc_handle, 0,
-                      config_ident, imapd_clienthost, imapd_userid,
-                      index_mboxname(imapd_index), NULL);
+        r = proc_register(&proc_handle, 0,
+                          config_ident, imapd_clienthost, imapd_userid,
+                          index_mboxname(imapd_index), NULL);
+        if (r) fatal("unable to register process", EX_IOERR);
         proc_settitle(config_ident, imapd_clienthost, imapd_userid,
                       index_mboxname(imapd_index), NULL);
 
@@ -1504,9 +1509,10 @@ static void cmdloop(void)
         if (config_getswitch(IMAPOPT_CHATTY))
             syslog(LOG_NOTICE, "command: %s %s", tag.s, cmd.s);
 
-        proc_register(&proc_handle, 0,
-                      config_ident, imapd_clienthost, imapd_userid,
-                      index_mboxname(imapd_index), cmd.s);
+        r = proc_register(&proc_handle, 0,
+                          config_ident, imapd_clienthost, imapd_userid,
+                          index_mboxname(imapd_index), cmd.s);
+        if (r) fatal("unable to register process", EX_IOERR);
         proc_settitle(config_ident, imapd_clienthost, imapd_userid,
                       index_mboxname(imapd_index), cmd.s);
 
diff --git a/imap/nntpd.c b/imap/nntpd.c
index f5140d96e5..2f1d7f1236 100644
--- a/imap/nntpd.c
+++ b/imap/nntpd.c
@@ -730,9 +730,10 @@ static void cmdloop(void)
 
         signals_poll();
 
-        proc_register(&proc_handle, 0,
-                      config_ident, nntp_clienthost, nntp_userid,
-                      index_mboxname(group_state), NULL);
+        r = proc_register(&proc_handle, 0,
+                          config_ident, nntp_clienthost, nntp_userid,
+                          index_mboxname(group_state), NULL);
+        if (r) fatal("unable to register process", EX_IOERR);
         proc_settitle(config_ident, nntp_clienthost, nntp_userid,
                       index_mboxname(group_state), NULL);
 
@@ -779,9 +780,10 @@ static void cmdloop(void)
             if (Uisupper(*p)) *p = tolower((unsigned char) *p);
         }
 
-        proc_register(&proc_handle, 0,
-                      config_ident, nntp_clienthost, nntp_userid,
-                      index_mboxname(group_state), cmd.s);
+        r = proc_register(&proc_handle, 0,
+                          config_ident, nntp_clienthost, nntp_userid,
+                          index_mboxname(group_state), cmd.s);
+        if (r) fatal("unable to register process", EX_IOERR);
         proc_settitle(config_ident, nntp_clienthost, nntp_userid,
                       index_mboxname(group_state), cmd.s);
 
diff --git a/imap/pop3d.c b/imap/pop3d.c
index 7127755c5f..095ab511a4 100644
--- a/imap/pop3d.c
+++ b/imap/pop3d.c
@@ -857,15 +857,17 @@ static void cmdloop(void)
     char *p;
     char *arg;
     uint32_t msgno = 0;
+    int r;
 
     for (;;) {
         signals_poll();
 
         /* register process */
-        proc_register(&proc_handle, 0,
-                      config_ident, popd_clienthost, popd_userid,
-                      popd_mailbox ? mailbox_name(popd_mailbox) : NULL,
-                      NULL);
+        r = proc_register(&proc_handle, 0,
+                          config_ident, popd_clienthost, popd_userid,
+                          popd_mailbox ? mailbox_name(popd_mailbox) : NULL,
+                          NULL);
+        if (r) fatal("unable to register process", EX_IOERR);
         proc_settitle(config_ident, popd_clienthost, popd_userid,
                       popd_mailbox ? mailbox_name(popd_mailbox) : NULL,
                       NULL);
@@ -947,10 +949,11 @@ static void cmdloop(void)
             syslog(LOG_NOTICE, "command: %s", inputbuf);
 
         /* register process */
-        proc_register(&proc_handle, 0,
-                      config_ident, popd_clienthost, popd_userid,
-                      popd_mailbox ? mailbox_name(popd_mailbox) : NULL,
-                      inputbuf);
+        r = proc_register(&proc_handle, 0,
+                          config_ident, popd_clienthost, popd_userid,
+                          popd_mailbox ? mailbox_name(popd_mailbox) : NULL,
+                          inputbuf);
+        if (r) fatal("unable to register process", EX_IOERR);
         proc_settitle(config_ident, popd_clienthost, popd_userid,
                       popd_mailbox ? mailbox_name(popd_mailbox) : NULL,
                       inputbuf);
diff --git a/imap/sync_server.c b/imap/sync_server.c
index 7e403c5fc9..5cf30eeb25 100644
--- a/imap/sync_server.c
+++ b/imap/sync_server.c
@@ -324,7 +324,7 @@ int service_main(int argc __attribute__((unused)),
 {
     const char *localip, *remoteip;
     sasl_security_properties_t *secprops = NULL;
-    int timeout;
+    int r, timeout;
 
     signals_poll();
 
@@ -366,8 +366,9 @@ int service_main(int argc __attribute__((unused)),
         tcp_disable_nagle(1); /* XXX magic fd */
     }
 
-    proc_register(&proc_handle, 0,
-                  config_ident, sync_clienthost, NULL, NULL, NULL);
+    r = proc_register(&proc_handle, 0,
+                      config_ident, sync_clienthost, NULL, NULL, NULL);
+    if (r) fatal("unable to register process", EX_IOERR);
     proc_settitle(config_ident, sync_clienthost, NULL, NULL, NULL);
 
     /* Set inactivity timer */
@@ -786,8 +787,9 @@ static void cmd_authenticate(char *mech, char *resp)
     }
 
     sync_userid = xstrdup((const char *) val);
-    proc_register(&proc_handle, 0,
-                  config_ident, sync_clienthost, sync_userid, NULL, NULL);
+    r = proc_register(&proc_handle, 0,
+                      config_ident, sync_clienthost, sync_userid, NULL, NULL);
+    if (r) fatal("unable to register process", EX_IOERR);
     proc_settitle(config_ident, sync_clienthost, sync_userid, NULL, NULL);
 
     syslog(LOG_NOTICE, "login: %s %s %s%s %s", sync_clienthost, sync_userid,
diff --git a/lib/proc.c b/lib/proc.c
index f13c53cf27..4a79fdce45 100644
--- a/lib/proc.c
+++ b/lib/proc.c
@@ -141,6 +141,7 @@ EXPORTED int proc_register(struct proc_handle **handlep,
     FILE *procfile = NULL;
     char *newfname = NULL;
     struct proc_handle *handle = NULL;
+    int handle_is_new = 0;
 
     assert(handlep != NULL);
 
@@ -150,6 +151,7 @@ EXPORTED int proc_register(struct proc_handle **handlep,
     }
     else {
         handle = xmalloc(sizeof *handle);
+        handle_is_new = 1;
         if (!pid) pid = getpid();
         handle->pid = pid;
         handle->fname = proc_getpath(pid, /*isnew*/0);
@@ -161,14 +163,17 @@ EXPORTED int proc_register(struct proc_handle **handlep,
     procfile = fopen(newfname, "w+");
     if (!procfile) {
         if (cyrus_mkdir(newfname, 0755) == -1) {
-            fatal("couldn't create proc directory", EX_IOERR);
+            xsyslog(LOG_ERR, "IOERROR: failed to create proc directory",
+                               "fname=<%s>", newfname);
+            goto error;
         }
         else {
             syslog(LOG_NOTICE, "created proc directory");
             procfile = fopen(newfname, "w+");
             if (!procfile) {
-                syslog(LOG_ERR, "IOERROR: creating %s: %m", newfname);
-                fatal("can't write proc file", EX_IOERR);
+                xsyslog(LOG_ERR, "IOERROR: failed to create proc file",
+                                 "fname=<%s>", newfname);
+                goto error;
             }
         }
     }
@@ -187,11 +192,21 @@ EXPORTED int proc_register(struct proc_handle **handlep,
                          "source=<%s> dest=<%s>",
                          newfname, handle->fname);
         xunlink(newfname);
-        fatal("can't write proc file", EX_IOERR);
+        goto error;
     }
 
     free(newfname);
     return 0;
+
+error:
+    if (handle_is_new) {
+        xunlink(handle->fname);
+        free(handle->fname);
+        free(handle);
+        *handlep = NULL;
+    }
+    free(newfname);
+    return -1;
 }
 
 EXPORTED void proc_cleanup(struct proc_handle **handlep)

From 7e4a7562eb7695d5ebdcd6c9823e284aff764937 Mon Sep 17 00:00:00 2001
From: ellie timoney <ellie@fastmail.com>
Date: Mon, 19 Jun 2023 10:10:10 +1000
Subject: [PATCH 07/16] Info: test preexisting 'cyr_info proc' behaviour

---
 cassandane/Cassandane/Cyrus/Info.pm | 56 +++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/cassandane/Cassandane/Cyrus/Info.pm b/cassandane/Cassandane/Cyrus/Info.pm
index 4b2605a5bb..916876ea72 100644
--- a/cassandane/Cassandane/Cyrus/Info.pm
+++ b/cassandane/Cassandane/Cyrus/Info.pm
@@ -211,4 +211,60 @@ sub test_info_lint_partitions
     );
 }
 
+sub test_proc_services
+{
+    my ($self) = @_;
+
+    # no clients => no service daemons => no processes
+    my @output = $self->run_cyr_info('proc');
+    $self->assert_num_equals(0, scalar @output);
+
+    # master spawns service processes when clients connect to them
+    my $imap_svc = $self->{instance}->get_service('imap');
+    my @clients;
+    foreach (1..5) {
+        # five concurrent connections for a single user is normal,
+        # e.g. thunderbird does this
+        my $store = $imap_svc->create_store(username => 'cassandane');
+        my $imaptalk = $store->get_client();
+        push @clients, $imaptalk if $imaptalk;
+    }
+
+    # better have got some clients from that!
+    $self->assert_num_gte(1, scalar @clients);
+
+    # five clients => five service daemons => five processes
+    @output = $self->run_cyr_info('proc');
+    $self->assert_num_equals(scalar @clients, scalar @output);
+
+    # log clients out one at a time, expect proc count to decrease
+    while (scalar @clients) {
+        my $old = shift @clients;
+        $old->logout();
+
+        @output = $self->run_cyr_info('proc');
+        $self->assert_num_equals(scalar @clients, scalar @output);
+    }
+}
+
+sub test_proc_starts
+    :NoStartInstances
+{
+    my ($self) = @_;
+
+    # we used to recommend starting idled from START, and it will
+    # still work like that, so using it here saves me mocking something
+    $self->{instance}->add_start(name => 'idled',
+                                 argv => [ 'idled' ]);
+    $self->{instance}->start();
+
+    # entries listed in START run to completion before master fully
+    # starts up.  if they fork themselves and hang around (like idled
+    # does) then that's their business, but master can't and doesn't
+    # track them
+    my @output = $self->run_cyr_info('proc');
+
+    $self->assert_num_equals(0, scalar @output);
+}
+
 1;

From 8014c88687387a572d4efd5dfe5cf4abd025c635 Mon Sep 17 00:00:00 2001
From: ellie timoney <ellie@fastmail.com>
Date: Mon, 19 Jun 2023 11:01:14 +1000
Subject: [PATCH 08/16] Master: fix and enable periodic_event_slow test

---
 cassandane/Cassandane/Cyrus/Master.pm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/cassandane/Cassandane/Cyrus/Master.pm b/cassandane/Cassandane/Cyrus/Master.pm
index 04e1d1f7ec..0faa78deca 100644
--- a/cassandane/Cassandane/Cyrus/Master.pm
+++ b/cassandane/Cassandane/Cyrus/Master.pm
@@ -845,7 +845,7 @@ sub test_maxforkrate
                               }, $self->lemming_census());
 }
 
-sub XXXtest_periodic_event
+sub test_periodic_event_slow
 {
     my ($self) = @_;
 
@@ -860,8 +860,8 @@ sub XXXtest_periodic_event
 
     xlog $self, "periodic events run immediately";
 
-    xlog $self, "waiting 5 mins for events to fire";
-    sleep(5*60);
+    xlog $self, "waiting 5 mins for events to fire, plus some slop";
+    sleep(5*60 + 5);
 
     $self->assert_deep_equals({
                                 B => { live => 0, dead => 6 },

From df0c9d3f02a70c12f240551877d2a87c95cdeb37 Mon Sep 17 00:00:00 2001
From: ellie timoney <ellie@fastmail.com>
Date: Fri, 9 Jun 2023 16:13:32 +1000
Subject: [PATCH 09/16] master: register DAEMON and EVENT processes

---
 master/master.c | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/master/master.c b/master/master.c
index 137af68f25..0eb5c20df4 100644
--- a/master/master.c
+++ b/master/master.c
@@ -95,11 +95,12 @@
 
 #include "assert.h"
 #include "cyr_lock.h"
+#include "proc.h"
 #include "retry.h"
+#include "strarray.h"
 #include "util.h"
 #include "xmalloc.h"
 #include "xunlink.h"
-#include "strarray.h"
 
 enum {
     child_table_size = 10000,
@@ -164,6 +165,7 @@ struct centry {
     char *desc;                 /* human readable description for logging */
     struct timeval spawntime;   /* when the centry was allocated */
     time_t sighuptime;          /* when did we send a SIGHUP */
+    struct proc_handle *proc_handle; /* for tracking proc registrations */
     struct centry *next;
 };
 static struct centry *ctable[child_table_size];
@@ -334,6 +336,8 @@ static char *centry_describe(const struct centry *c, pid_t pid)
 /* free a centry */
 static void centry_free(struct centry *c)
 {
+    assert(c->proc_handle == NULL); /* better have cleaned this up already */
+
     free(c->desc);
     free(c);
 }
@@ -1026,6 +1030,14 @@ static void spawn_waitdaemon(struct service *s, int wdi)
         c->si = SERVICE_NONE;
         c->wdi = wdi;
         centry_set_state(c, SERVICE_STATE_READY);
+        r = proc_register(&c->proc_handle, p,
+                          s->name, NULL, NULL, NULL, NULL);
+        if (r) {
+            syslog(LOG_ERR, "ERROR: unable to register process %u"
+                            " for waitdaemon %s/%s",
+                            p, s->name, s->familyname);
+            r = 0; /* don't sweat it */
+        }
         centry_add(c, p);
     }
     else {
@@ -1197,6 +1209,17 @@ static void spawn_service(struct service *s, int si, int wdi)
         c->si = si;
         c->wdi = wdi;
         centry_set_state(c, SERVICE_STATE_READY);
+        if (!s->listen) {
+            /* we only register DAEMONs -- SERVICEs register themselves */
+            r = proc_register(&c->proc_handle, p,
+                              s->name, NULL, NULL, NULL, NULL);
+            if (r) {
+                syslog(LOG_ERR, "ERROR: unable to register process %u"
+                                " for daemon %s/%s",
+                                p, s->name, s->familyname);
+                r = 0; /* don't sweat it */
+            }
+        }
         centry_add(c, p);
         break;
     }
@@ -1228,7 +1251,7 @@ static void schedule_event(struct event *a)
 static void spawn_schedule(struct timeval now)
 {
     struct event *a, *b;
-    int i;
+    int i, r;
     char path[PATH_MAX];
     pid_t p;
     struct centry *c;
@@ -1290,6 +1313,13 @@ static void spawn_schedule(struct timeval now)
                 c = centry_alloc();
                 centry_set_name(c, "EVENT", a->name, path);
                 centry_set_state(c, SERVICE_STATE_READY);
+                r = proc_register(&c->proc_handle, p,
+                                  a->name, NULL, NULL, NULL, NULL);
+                if (r) {
+                    syslog(LOG_ERR, "ERROR: unable to register process %u"
+                                    " for event %s",
+                                    p, a->name);
+                }
                 centry_add(c, p);
                 break;
             }
@@ -1477,6 +1507,8 @@ static void reap_child(void)
                            pid, c->service_state);
                 }
             }
+
+            proc_cleanup(&c->proc_handle);
             centry_set_state(c, SERVICE_STATE_DEAD);
         } else {
             /* Are we multithreaded now? we don't know this child */

From 8aef74dfaad0a79b0df12bb9611ca5b472dfc7c5 Mon Sep 17 00:00:00 2001
From: ellie timoney <ellie@fastmail.com>
Date: Mon, 19 Jun 2023 12:21:31 +1000
Subject: [PATCH 10/16] Info: test 'cyr_info proc' with periodic events

---
 cassandane/Cassandane/Cyrus/Info.pm | 42 +++++++++++++++++++++++++++++
 cassandane/utils/sleeper            | 31 +++++++++++++++++++++
 2 files changed, 73 insertions(+)
 create mode 100755 cassandane/utils/sleeper

diff --git a/cassandane/Cassandane/Cyrus/Info.pm b/cassandane/Cassandane/Cyrus/Info.pm
index 916876ea72..d1173c4202 100644
--- a/cassandane/Cassandane/Cyrus/Info.pm
+++ b/cassandane/Cassandane/Cyrus/Info.pm
@@ -40,6 +40,7 @@
 package Cassandane::Cyrus::Info;
 use strict;
 use warnings;
+use Cwd qw(realpath);
 use Data::Dumper;
 
 use lib '.';
@@ -267,4 +268,45 @@ sub test_proc_starts
     $self->assert_num_equals(0, scalar @output);
 }
 
+sub test_proc_periodic_events_slow
+    :NoStartInstances
+{
+    my ($self) = @_;
+
+    my $sleeper_time = 10; # seconds
+
+    # periodic events first fire immediately at startup, and then every
+    # 'period' minutes thereafter. the fastest we can schedule them is
+    # every 1 minute, so this test must run for at least several real
+    # minutes
+    $self->{instance}->add_event(
+        name => 'sleeper',
+        argv => [ realpath('utils/sleeper'), $sleeper_time ],
+        period => 1,
+    );
+    $self->{instance}->start();
+
+    sleep 2; # offset our checks a little to avoid races
+
+    # observe for three cycles
+    my $observations = 3;
+    while ($observations > 0) {
+        # event should have fired and be running
+        my @output = $self->run_cyr_info('proc');
+        $self->assert_num_equals(1, scalar @output);
+
+        # wait for it to finish and check again
+        sleep $sleeper_time;
+        @output = $self->run_cyr_info('proc');
+        $self->assert_num_equals(0, scalar @output);
+
+        # skip final wait if we're done
+        $observations--;
+        last if $observations == 0;
+
+        # wait until next period
+        sleep 60 - $sleeper_time;
+    }
+}
+
 1;
diff --git a/cassandane/utils/sleeper b/cassandane/utils/sleeper
new file mode 100755
index 0000000000..0035fdb804
--- /dev/null
+++ b/cassandane/utils/sleeper
@@ -0,0 +1,31 @@
+#!/usr/bin/perl
+# like /bin/sleep except ignoring Cyrus-style -C, -M arguments
+
+use warnings;
+use strict;
+
+use Sys::Syslog qw(:standard :macros);
+
+sub usage
+{
+    die "usage: $0 [-C imapd.conf] [-M cyrus.conf] seconds\n";
+}
+
+my $arg;
+
+while (scalar @ARGV > 1) {
+    $arg = shift @ARGV;
+    if ($arg eq '-C' || $arg eq '-M') {
+        # ignore argument intended for cyrus processes
+        shift @ARGV;
+    }
+}
+usage() if scalar @ARGV != 1;
+
+$arg = shift @ARGV;
+usage() if $arg !~ m/^\d+$/;
+
+openlog('sleeper', 'pid', LOG_LOCAL6) or die "Cannot openlog";
+syslog(LOG_INFO, "sleeping for $arg seconds...");
+sleep $arg;
+syslog(LOG_INFO, "finished");

From c307c866d43134ffc0417368377e7e80294616c7 Mon Sep 17 00:00:00 2001
From: ellie timoney <ellie@fastmail.com>
Date: Mon, 19 Jun 2023 13:59:41 +1000
Subject: [PATCH 11/16] Info: test 'cyr_info proc' with scheduled events

---
 cassandane/Cassandane/Cyrus/Info.pm | 36 +++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/cassandane/Cassandane/Cyrus/Info.pm b/cassandane/Cassandane/Cyrus/Info.pm
index d1173c4202..5e9e512ae1 100644
--- a/cassandane/Cassandane/Cyrus/Info.pm
+++ b/cassandane/Cassandane/Cyrus/Info.pm
@@ -42,6 +42,7 @@ use strict;
 use warnings;
 use Cwd qw(realpath);
 use Data::Dumper;
+use Date::Format qw(time2str);
 
 use lib '.';
 use base qw(Cassandane::Cyrus::TestCase);
@@ -309,4 +310,39 @@ sub test_proc_periodic_events_slow
     }
 }
 
+sub test_proc_scheduled_events
+    :NoStartInstances
+{
+    my ($self) = @_;
+
+    my $sleeper_time = 10;
+
+    # schedule an event to fire at the next minute boundary that is at
+    # least ten seconds away
+    my $at = time + 70;
+    $at -= ($at % 60);
+    my $at_hm = time2str('%H%M', $at);
+    xlog $self, "scheduling event to run at $at_hm ($at)";
+    $self->{instance}->add_event(
+        name => 'sleeper',
+        argv => [ realpath('utils/sleeper'), $sleeper_time ],
+        at => $at_hm,
+    );
+    $self->{instance}->start();
+
+    # event process should not be running at startup
+    my @output = $self->run_cyr_info('proc');
+    $self->assert_num_equals(0, scalar @output);
+
+    # should be running at the scheduled time (with a little slop)
+    sleep 2 + $at - time;
+    @output = $self->run_cyr_info('proc');
+    $self->assert_num_equals(1, scalar @output);
+
+    # should not be running after we expect it to have finished
+    sleep $sleeper_time;
+    @output = $self->run_cyr_info('proc');
+    $self->assert_num_equals(0, scalar @output);
+}
+
 1;

From 6830341f427da45d8bb444ece2d29323506908e5 Mon Sep 17 00:00:00 2001
From: ellie timoney <ellie@fastmail.com>
Date: Wed, 21 Jun 2023 10:14:07 +1000
Subject: [PATCH 12/16] GenericDaemon: rename to GenericListener

The defining property of these objects is that they are listening
on a port, which conflicts with Cyrus's use of DAEMON for things
that are not listening on a port.
---
 cassandane/Cassandane/Cyrus/Annotator.pm      |  6 +--
 cassandane/Cassandane/Cyrus/TestCase.pm       |  3 +-
 .../{GenericDaemon.pm => GenericListener.pm}  |  4 +-
 cassandane/Cassandane/Instance.pm             | 44 +++++++++----------
 cassandane/Cassandane/Service.pm              | 27 ++++++------
 5 files changed, 42 insertions(+), 42 deletions(-)
 rename cassandane/Cassandane/{GenericDaemon.pm => GenericListener.pm} (99%)

diff --git a/cassandane/Cassandane/Cyrus/Annotator.pm b/cassandane/Cassandane/Cyrus/Annotator.pm
index 36a305c7b9..d51450c75b 100644
--- a/cassandane/Cassandane/Cyrus/Annotator.pm
+++ b/cassandane/Cassandane/Cyrus/Annotator.pm
@@ -78,14 +78,14 @@ sub start_my_instances
 {
     my ($self) = @_;
 
-    $self->{instance}->add_generic_daemon(
+    $self->{instance}->add_generic_listener(
         name => 'annotator',
         port => $self->{instance}->{config}->get('annotation_callout'),
         argv => sub {
-            my ($daemon) = @_;
+            my ($listener) = @_;
             return (
                 abs_path('utils/annotator.pl'),
-                '--port', $daemon->port(),
+                '--port', $listener->port(),
                 '--pidfile', '@basedir@/run/annotator.pid',
                 );
         });
diff --git a/cassandane/Cassandane/Cyrus/TestCase.pm b/cassandane/Cassandane/Cyrus/TestCase.pm
index b3acdfb05e..1db94b2d8a 100644
--- a/cassandane/Cassandane/Cyrus/TestCase.pm
+++ b/cassandane/Cassandane/Cyrus/TestCase.pm
@@ -53,6 +53,7 @@ use base qw(Cassandane::Unit::TestCase);
 use Cassandane::Util::Log;
 use Cassandane::Util::Words;
 use Cassandane::Generator;
+use Cassandane::GenericListener;
 use Cassandane::MessageStoreFactory;
 use Cassandane::Instance;
 use Cassandane::PortManager;
@@ -1026,7 +1027,7 @@ sub post_tear_down
     }
 
     die "Found some stray processes"
-        if (Cassandane::GenericDaemon::kill_processes_on_ports(
+        if (Cassandane::GenericListener::kill_processes_on_ports(
                     Cassandane::PortManager::free_all()));
 }
 
diff --git a/cassandane/Cassandane/GenericDaemon.pm b/cassandane/Cassandane/GenericListener.pm
similarity index 99%
rename from cassandane/Cassandane/GenericDaemon.pm
rename to cassandane/Cassandane/GenericListener.pm
index d44ba05d74..1b59f7aa1e 100644
--- a/cassandane/Cassandane/GenericDaemon.pm
+++ b/cassandane/Cassandane/GenericListener.pm
@@ -37,7 +37,7 @@
 #  OF THIS SOFTWARE.
 #
 
-package Cassandane::GenericDaemon;
+package Cassandane::GenericListener;
 use strict;
 use warnings;
 
@@ -108,7 +108,7 @@ sub set_port
     $self->{port} = $port;
 }
 
-# Return a hash of parameters for connecting to the daemon.
+# Return a hash of parameters for connecting to the listener.
 # These will ultimately go through to MessageStoreFactory::create.
 sub connection_params
 {
diff --git a/cassandane/Cassandane/Instance.pm b/cassandane/Cassandane/Instance.pm
index 27ea6bbcbb..07529592dd 100644
--- a/cassandane/Cassandane/Instance.pm
+++ b/cassandane/Cassandane/Instance.pm
@@ -71,7 +71,7 @@ use Cassandane::Mboxname;
 use Cassandane::Config;
 use Cassandane::Service;
 use Cassandane::ServiceFactory;
-use Cassandane::GenericDaemon;
+use Cassandane::GenericListener;
 use Cassandane::MasterStart;
 use Cassandane::MasterEvent;
 use Cassandane::Cassini;
@@ -104,7 +104,7 @@ sub new
         starts => [],
         services => {},
         events => [],
-        generic_daemons => {},
+        generic_listeners => {},
         re_use_dir => 0,
         setup_mailbox => 1,
         persistent => 0,
@@ -465,24 +465,24 @@ sub add_event
     push(@{$self->{events}}, Cassandane::MasterEvent->new(%params));
 }
 
-sub add_generic_daemon
+sub add_generic_listener
 {
     my ($self, %params) = @_;
 
     my $name = delete $params{name};
     die "Missing parameter 'name'"
         unless defined $name;
-    die "Already have a generic daemon named \"$name\""
-        if defined $self->{generic_daemons}->{$name};
+    die "Already have a generic listener named \"$name\""
+        if defined $self->{generic_listeners}->{$name};
 
-    my $daemon = Cassandane::GenericDaemon->new(
-            name => $name,
-            config => $self->{config},
-            %params
+    my $listener = Cassandane::GenericListener->new(
+        name => $name,
+        config => $self->{config},
+        %params
     );
 
-    $self->{generic_daemons}->{$name} = $daemon;
-    return $daemon;
+    $self->{generic_listeners}->{$name} = $listener;
+    return $listener;
 }
 
 sub set_config
@@ -491,7 +491,7 @@ sub set_config
 
     $self->{config} = $conf;
     map { $_->set_config($conf); } (values %{$self->{services}},
-                                    values %{$self->{generic_daemons}});
+                                    values %{$self->{generic_listeners}});
 }
 
 sub _find_binary
@@ -786,8 +786,6 @@ sub _generate_master_conf
         print MASTER "}\n";
     }
 
-    # $self->{generic_daemons} is daemons *not* managed by master
-
     close MASTER;
 }
 
@@ -837,7 +835,7 @@ sub _add_services_from_cyrus_conf
 
             if ($k eq 'listen')
             {
-                my $aa = Cassandane::GenericDaemon::parse_address($v);
+                my $aa = Cassandane::GenericListener::parse_address($v);
                 $params{host} = $aa->{host};
                 $params{port} = $aa->{port};
             }
@@ -908,7 +906,7 @@ sub _start_master
     # a second set of Cassandane tests on this machine, which is
     # also going to fail miserably.  In any case we want to know.
     foreach my $srv (values %{$self->{services}},
-                     values %{$self->{generic_daemons}})
+                     values %{$self->{generic_listeners}})
     {
         die "Some process is already listening on " . $srv->address()
             if $srv->is_listening();
@@ -941,10 +939,10 @@ sub _start_master
                 description => "the master PID file to exist");
     xlog "_start_master: PID file present and correct";
 
-    # Start any other defined daemons
-    foreach my $daemon (values %{$self->{generic_daemons}})
+    # Start any other defined listeners
+    foreach my $listener (values %{$self->{generic_listeners}})
     {
-        $self->run_command({ cyrus => 0 }, $daemon->get_argv());
+        $self->run_command({ cyrus => 0 }, $listener->get_argv());
     }
 
     # Wait until all the defined services are reported as listening.
@@ -953,7 +951,7 @@ sub _start_master
     # might be a bit slow.
     xlog "_start_master: PID waiting for services";
     foreach my $srv (values %{$self->{services}},
-                     values %{$self->{generic_daemons}})
+                     values %{$self->{generic_listeners}})
     {
         timed_wait(sub
                 {
@@ -1987,11 +1985,11 @@ sub describe
         printf "        ";
         $srv->describe();
     }
-    printf "    generic daemons:\n";
-    foreach my $daemon (values %{$self->{generic_daemons}})
+    printf "    generic listeners:\n";
+    foreach my $listener (values %{$self->{generic_listeners}})
     {
         printf "        ";
-        $daemon->describe();
+        $listener->describe();
     }
 }
 
diff --git a/cassandane/Cassandane/Service.pm b/cassandane/Cassandane/Service.pm
index 4e44b18c0a..80fbd104a5 100644
--- a/cassandane/Cassandane/Service.pm
+++ b/cassandane/Cassandane/Service.pm
@@ -43,7 +43,7 @@ use warnings;
 
 use lib '.';
 use base qw(Cassandane::MasterEntry);
-use Cassandane::GenericDaemon;
+use Cassandane::GenericListener;
 use Cassandane::Util::Log;
 use Cassandane::MessageStoreFactory;
 use Cassandane::Util::Socket;
@@ -60,9 +60,10 @@ sub new
 
     my $self = $class->SUPER::new(%params);
 
-    $self->{_daemon} = Cassandane::GenericDaemon->new(name => $params{name},
-                                                      host => $host,
-                                                      port => $port);
+    $self->{_listener} = Cassandane::GenericListener->new(
+                            name => $params{name},
+                            host => $host,
+                            port => $port);
     $self->{type} = $type;
 
     return $self;
@@ -78,27 +79,27 @@ sub set_config
 {
     my ($self, $config) = @_;
     $self->SUPER::set_config($config);
-    $self->{_daemon}->set_config($config);
+    $self->{_listener}->set_config($config);
 }
 
 # Return the host
 sub host
 {
     my ($self) = @_;
-    return $self->{_daemon}->host();
+    return $self->{_listener}->host();
 }
 
 # Return the port
 sub port
 {
     my ($self) = @_;
-    return $self->{_daemon}->port();
+    return $self->{_listener}->port();
 }
 
 sub set_port
 {
     my ($self, $port) = @_;
-    return $self->{_daemon}->set_port($port);
+    return $self->{_listener}->set_port($port);
 }
 
 # Return a hash of parameters suitable for passing
@@ -107,7 +108,7 @@ sub store_params
 {
     my ($self, %params) = @_;
 
-    my $pp = $self->{_daemon}->connection_params(%params);
+    my $pp = $self->{_listener}->connection_params(%params);
     $pp->{type} ||= $self->{type};
     $pp->{username} ||= 'cassandane';
     $pp->{password} ||= 'testpw';
@@ -143,25 +144,25 @@ sub master_params
 sub address
 {
     my ($self) = @_;
-    return $self->{_daemon}->address();
+    return $self->{_listener}->address();
 }
 
 sub address_family
 {
     my ($self) = @_;
-    return $self->{_daemon}->address_family();
+    return $self->{_listener}->address_family();
 }
 
 sub is_listening
 {
     my ($self) = @_;
-    return $self->{_daemon}->is_listening();
+    return $self->{_listener}->is_listening();
 }
 
 sub describe
 {
     my ($self) = @_;
-    $self->{_daemon}->describe();
+    $self->{_listener}->describe();
 }
 
 

From 10ff8d45a86189905db749864a10e3b3241aec99 Mon Sep 17 00:00:00 2001
From: ellie timoney <ellie@fastmail.com>
Date: Wed, 21 Jun 2023 11:49:42 +1000
Subject: [PATCH 13/16] Instance: add support for cyrus.conf DAEMON entries

---
 cassandane/Cassandane/Instance.pm     | 30 +++++++++++++-
 cassandane/Cassandane/MasterDaemon.pm | 58 +++++++++++++++++++++++++++
 2 files changed, 87 insertions(+), 1 deletion(-)
 create mode 100644 cassandane/Cassandane/MasterDaemon.pm

diff --git a/cassandane/Cassandane/Instance.pm b/cassandane/Cassandane/Instance.pm
index 07529592dd..a9c7f4d98a 100644
--- a/cassandane/Cassandane/Instance.pm
+++ b/cassandane/Cassandane/Instance.pm
@@ -74,6 +74,7 @@ use Cassandane::ServiceFactory;
 use Cassandane::GenericListener;
 use Cassandane::MasterStart;
 use Cassandane::MasterEvent;
+use Cassandane::MasterDaemon;
 use Cassandane::Cassini;
 use Cassandane::PortManager;
 use Cassandane::Net::SMTPServer;
@@ -104,6 +105,7 @@ sub new
         starts => [],
         services => {},
         events => [],
+        daemons => {},
         generic_listeners => {},
         re_use_dir => 0,
         setup_mailbox => 1,
@@ -465,6 +467,19 @@ sub add_event
     push(@{$self->{events}}, Cassandane::MasterEvent->new(%params));
 }
 
+sub add_daemon
+{
+    my ($self, %params) = @_;
+
+    my $name = $params{name};
+    die "Missing parameter 'name'"
+        unless defined $name;
+    die "Already have a daemon named \"$name\""
+        if defined $self->{daemons}->{$name};
+
+    $self->{daemons}->{$name} = Cassandane::MasterDaemon->new(%params);
+}
+
 sub add_generic_listener
 {
     my ($self, %params) = @_;
@@ -786,6 +801,13 @@ sub _generate_master_conf
         print MASTER "}\n";
     }
 
+    if (scalar %{$self->{daemons}})
+    {
+        print MASTER "DAEMON {\n";
+        $self->_emit_master_entry($_) for values %{$self->{daemons}};
+        print MASTER "}\n";
+    }
+
     close MASTER;
 }
 
@@ -803,7 +825,7 @@ sub _add_services_from_cyrus_conf
         chomp;
         s/\s*#.*//;             # strip comments
         next if m/^\s*$/;       # skip empty lines
-        my ($m) = m/^(START|SERVICES|EVENTS)\s*{/;
+        my ($m) = m/^(START|SERVICES|EVENTS|DAEMON)\s*{/;
         if ($m)
         {
             $in = $m;
@@ -1207,6 +1229,10 @@ sub start
     elsif (!scalar $self->{services})
     {
         $self->_add_services_from_cyrus_conf();
+        # XXX START, EVENTS, DAEMON entries will be missed here if reusing
+        # XXX the directory.  Does it matter?  Maybe not, since the master
+        # XXX conf already contains them, so they'll still run, just
+        # XXX cassandane won't know about it.
     }
     $self->setup_syslog_replacement();
     $self->_start_notifyd();
@@ -1217,6 +1243,8 @@ sub start
 
     # give fakesaslauthd a moment (but not more than 2s) to set up its
     # socket before anything starts trying to connect to services
+    # XXX if this were a DAEMON with wait=y, this would be unnecessary,
+    # XXX though those didn't exist until 3.4
     if ($self->{authdaemon}) {
         my $tries = 0;
         while (not -S $fakesaslauthd_socket && $tries < 2_000_000) {
diff --git a/cassandane/Cassandane/MasterDaemon.pm b/cassandane/Cassandane/MasterDaemon.pm
new file mode 100644
index 0000000000..379cd66a25
--- /dev/null
+++ b/cassandane/Cassandane/MasterDaemon.pm
@@ -0,0 +1,58 @@
+#!/usr/bin/perl
+#
+#  Copyright (c) 2011-2023 FastMail Pty Ltd. All rights reserved.
+#
+#  Redistribution and use in source and binary forms, with or without
+#  modification, are permitted provided that the following conditions
+#  are met:
+#
+#  1. Redistributions of source code must retain the above copyright
+#     notice, this list of conditions and the following disclaimer.
+#
+#  2. Redistributions in binary form must reproduce the above copyright
+#     notice, this list of conditions and the following disclaimer in
+#     the documentation and/or other materials provided with the
+#     distribution.
+#
+#  3. The name "Fastmail Pty Ltd" must not be used to
+#     endorse or promote products derived from this software without
+#     prior written permission. For permission or any legal
+#     details, please contact
+#      FastMail Pty Ltd
+#      PO Box 234
+#      Collins St West 8007
+#      Victoria
+#      Australia
+#
+#  4. Redistributions of any form whatsoever must retain the following
+#     acknowledgment:
+#     "This product includes software developed by Fastmail Pty. Ltd."
+#
+#  FASTMAIL PTY LTD DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+#  INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY  AND FITNESS, IN NO
+#  EVENT SHALL OPERA SOFTWARE AUSTRALIA BE LIABLE FOR ANY SPECIAL, INDIRECT
+#  OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF
+#  USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+#  TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+#  OF THIS SOFTWARE.
+#
+
+package Cassandane::MasterDaemon;
+use strict;
+use warnings;
+
+use lib '.';
+use base qw(Cassandane::MasterEntry);
+
+sub new
+{
+    return shift->SUPER::new(@_);
+}
+
+sub _otherparams
+{
+    my ($self) = @_;
+    return ( qw(wait) );
+}
+
+1;

From ea553b5beecd938ab24e1493ad76bbc24507b598 Mon Sep 17 00:00:00 2001
From: ellie timoney <ellie@fastmail.com>
Date: Wed, 21 Jun 2023 11:59:03 +1000
Subject: [PATCH 14/16] Instance: run fakesaslauthd as a wait=y DAEMON

not strictly related to this PR, but I've been wanting to do this
for ages, and now I can, and it shows that the previous commit works
---
 cassandane/Cassandane/Cyrus/Info.pm |  5 +++++
 cassandane/Cassandane/Instance.pm   | 35 ++++++++++++++++++++---------
 cassandane/utils/fakesaslauthd      | 19 +++++++++++++++-
 3 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/cassandane/Cassandane/Cyrus/Info.pm b/cassandane/Cassandane/Cyrus/Info.pm
index 5e9e512ae1..4047246ba2 100644
--- a/cassandane/Cassandane/Cyrus/Info.pm
+++ b/cassandane/Cassandane/Cyrus/Info.pm
@@ -88,6 +88,11 @@ sub run_cyr_info
     my @res = readline(RESULTS);
     close RESULTS;
 
+    if ($args[0] eq 'proc') {
+        # if we see our fakesaslauthd, no we didn't
+        @res = grep { $_ !~ m/\bfakesaslauthd\b/ } @res;
+    }
+
     return @res;
 }
 
diff --git a/cassandane/Cassandane/Instance.pm b/cassandane/Cassandane/Instance.pm
index a9c7f4d98a..00a2964da0 100644
--- a/cassandane/Cassandane/Instance.pm
+++ b/cassandane/Cassandane/Instance.pm
@@ -1200,16 +1200,31 @@ sub start
     }
 
     # arrange for fakesaslauthd to be started by master
-    # XXX make this run as a DAEMON rather than a START
     my $fakesaslauthd_socket = "$self->{basedir}/run/mux";
+    my $fakesaslauthd_isdaemon = 1;
     if ($self->{authdaemon}) {
-        $self->add_start(
-            name => 'fakesaslauthd',
-            argv => [
-                abs_path('utils/fakesaslauthd'),
-                '-p', $fakesaslauthd_socket,
-            ],
-        );
+        my ($maj, $min) = Cassandane::Instance->get_version(
+                            $self->{installation});
+        if ($maj < 3 || ($maj == 3 && $min < 4)) {
+            $self->add_start(
+                name => 'fakesaslauthd',
+                argv => [
+                    abs_path('utils/fakesaslauthd'),
+                    '-p', $fakesaslauthd_socket,
+                ],
+            );
+            $fakesaslauthd_isdaemon = 0;
+        }
+        elsif (not exists $self->{daemons}->{fakesaslauthd}) {
+            $self->add_daemon(
+                name => 'fakesaslauthd',
+                argv => [
+                    abs_path('utils/fakesaslauthd'),
+                    '-p', $fakesaslauthd_socket,
+                ],
+                wait => 'y',
+            );
+        }
     }
 
     if (!$self->{re_use_dir} || ! -d $self->{basedir})
@@ -1243,9 +1258,7 @@ sub start
 
     # give fakesaslauthd a moment (but not more than 2s) to set up its
     # socket before anything starts trying to connect to services
-    # XXX if this were a DAEMON with wait=y, this would be unnecessary,
-    # XXX though those didn't exist until 3.4
-    if ($self->{authdaemon}) {
+    if ($self->{authdaemon} && !$fakesaslauthd_isdaemon) {
         my $tries = 0;
         while (not -S $fakesaslauthd_socket && $tries < 2_000_000) {
             $tries += usleep(10_000); # 10ms as us
diff --git a/cassandane/utils/fakesaslauthd b/cassandane/utils/fakesaslauthd
index 1cdb1a0277..af64fdc68b 100755
--- a/cassandane/utils/fakesaslauthd
+++ b/cassandane/utils/fakesaslauthd
@@ -3,6 +3,7 @@
 # for any user.  Use me for testing!
 
 use Getopt::Std;
+use IO::Handle;
 use IO::Socket::UNIX;
 use Sys::Syslog qw(:standard :macros);
 
@@ -16,13 +17,22 @@ sub get_counted_string
     return unpack("A$size", $data);
 }
 
+# support running as a DAEMON with wait=y:
+# * if fd 3 is already open, then we will need to write to it later to
+#   indicate we're ready.
+# * we must grab this early, before the number gets used for something
+#   else, otherwise we won't be able to differentiate between the fd 3
+#   we care about or some other thing
+# * if fd 3 was not already open, $status_fd will be undef
+my $status_fd = IO::Handle->new_from_fd(3, 'w');
+
 my %opts;
 
 getopts("C:dp:v", \%opts);
 
 die "need a socket path" if not $opts{p};
 
-openlog('fakesaslauthd', '', LOG_LOCAL6)
+openlog('fakesaslauthd', 'pid', LOG_LOCAL6)
     or die "Cannot openlog";
 
 # ok, we're good. background ourselves
@@ -46,6 +56,13 @@ syslog LOG_INFO, "listening on $opts{p}";
 my $shutdown = 0;
 $SIG{HUP} = sub { $shutdown++; };
 
+# okay, now we're ready to accept requests.  inform our parent,
+# if they were waiting to be informed
+if ($ENV{CYRUS_ISDAEMON} && $status_fd) {
+    print $status_fd "ok\r\n";
+    undef $status_fd;
+}
+
 while (my $client = $sock->accept()) {
     my $LoginName = get_counted_string($client);
     my $Password = get_counted_string($client);

From 6ba241fad2254a756d15395d7849d99cfb00647c Mon Sep 17 00:00:00 2001
From: ellie timoney <ellie@fastmail.com>
Date: Fri, 23 Jun 2023 10:47:29 +1000
Subject: [PATCH 15/16] Info: test 'cyr_info proc' with daemons

---
 cassandane/Cassandane/Cyrus/Info.pm | 46 +++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/cassandane/Cassandane/Cyrus/Info.pm b/cassandane/Cassandane/Cyrus/Info.pm
index 4047246ba2..7c1c53b091 100644
--- a/cassandane/Cassandane/Cyrus/Info.pm
+++ b/cassandane/Cassandane/Cyrus/Info.pm
@@ -350,4 +350,50 @@ sub test_proc_scheduled_events
     $self->assert_num_equals(0, scalar @output);
 }
 
+sub test_proc_daemons
+    :NoStartInstances
+{
+    my ($self) = @_;
+
+    my $sleeper_time = 10; # seconds
+    my $daemons = 3;
+
+    for my $i (1 .. $daemons) {
+        # you wouldn't usually run a daemon that exits and needs to be
+        # restarted every ten seconds, but it's useful for testing
+        # that cyr_info proc notices the pid changing
+        $self->{instance}->add_daemon(
+            name => "sleeper$i",
+            argv => [ realpath('utils/sleeper'), $sleeper_time ],
+        );
+    }
+    $self->{instance}->start();
+
+    sleep 2; # offset our checks a little to avoid races
+
+    my $observations = 3;
+    my %lastpid = map {; "sleeper$_" => 0 } (1 .. $daemons);
+    while ($observations > 0) {
+        my @output = $self->run_cyr_info('proc');
+
+        # always exactly one process per daemon
+        $self->assert_num_equals($daemons, scalar @output);
+
+        # expect a new pid for each daemon each time
+        foreach my $line (@output) {
+            my ($pid, $servicename, $host, $user, $mailbox, $cmd)
+                = split /\s/, $line, 6;
+            $self->assert_num_not_equals($lastpid{$servicename}, $pid);
+            $lastpid{$servicename} = $pid;
+        }
+
+        # skip final wait if we're done
+        $observations--;
+        last if $observations == 0;
+
+        # wait for next restart
+        sleep $sleeper_time;
+    }
+}
+
 1;

From 934a332de294cd9709590ddb650169689eaf930d Mon Sep 17 00:00:00 2001
From: ellie timoney <ellie@fastmail.com>
Date: Mon, 26 Jun 2023 15:58:41 +1000
Subject: [PATCH 16/16] docs: document changes to 'cyr_info proc' output

---
 changes/next/proc-info-for-daemons             | 18 ++++++++++++++++++
 .../manpages/systemcommands/cyr_info.rst       |  6 +++---
 .../manpages/systemcommands/master.rst         |  5 ++++-
 3 files changed, 25 insertions(+), 4 deletions(-)
 create mode 100644 changes/next/proc-info-for-daemons

diff --git a/changes/next/proc-info-for-daemons b/changes/next/proc-info-for-daemons
new file mode 100644
index 0000000000..f252138030
--- /dev/null
+++ b/changes/next/proc-info-for-daemons
@@ -0,0 +1,18 @@
+Description:
+
+Extends `cyr_info proc` to report DAEMON and EVENTS processes.
+
+
+Config changes:
+
+None
+
+
+Upgrade instructions:
+
+None
+
+
+GitHub issue:
+
+None
diff --git a/docsrc/imap/reference/manpages/systemcommands/cyr_info.rst b/docsrc/imap/reference/manpages/systemcommands/cyr_info.rst
index fc5cf5661d..04bc46ff1d 100644
--- a/docsrc/imap/reference/manpages/systemcommands/cyr_info.rst
+++ b/docsrc/imap/reference/manpages/systemcommands/cyr_info.rst
@@ -66,7 +66,7 @@ configuring Cyrus easier.
 
 .. option:: proc
 
-    Print all currently connected processes in the proc directory
+    Print active processes that :cyrusman:`master(8)` is managing.
 
 Options
 =======
@@ -103,7 +103,7 @@ Examples
 
 ..
 
-        List all the proc files and who they're logged in as.
+        List the active processes that master is managing
 
 .. only:: html
 
@@ -145,4 +145,4 @@ Files
 
 See Also
 ========
-:cyrusman:`imapd.conf(5)`, :cyrusman:`cyrus.conf(5)`
+:cyrusman:`imapd.conf(5)`, :cyrusman:`cyrus.conf(5)`, :cyrusman:`master(8)`
diff --git a/docsrc/imap/reference/manpages/systemcommands/master.rst b/docsrc/imap/reference/manpages/systemcommands/master.rst
index 75ff538a79..161c673742 100644
--- a/docsrc/imap/reference/manpages/systemcommands/master.rst
+++ b/docsrc/imap/reference/manpages/systemcommands/master.rst
@@ -113,6 +113,9 @@ The environment variable **CYRUS_VERBOSE** can be set to log additional
 debugging information. Setting the value to 1 results in base level logging.
 Setting it higher results in more log messages being generated.
 
+The :cyrusman:`cyr_info(8)` utility's ``proc`` subcommand can be used to
+list the active processes that **master** is managing.
+
 Files
 =====
 
@@ -125,4 +128,4 @@ See Also
 
 :cyrusman:`cyrus.conf(5)`, :cyrusman:`imapd.conf(5)`, :cyrusman:`imapd(8)`,
 :cyrusman:`pop3d(8)`, :cyrusman:`lmtpd(8)`, :cyrusman:`timsieved(8)`,
-:cyrusman:`idled(8)`
+:cyrusman:`idled(8)`, :cyrusman:`cyr_info(8)`