ctld: Move the pidfile handle out to a global variable

This ensures it will be destroyed (removing the associated pidfile)
anytime the process exits, including from exit(3) calls.  This fixes
a few places that would "leak" the pidfile on certain errors.

This also removes the need for some convoluted logic where
configuration objects would hand-off ownership of the pidfile handle
from the old configuration to the new configuration.

Reviewed by:	asomers
Sponsored by:	Chelsio Communications
Differential Revision:	https://reviews.freebsd.org/D56527

2 files changed, 14 insertions, 35 deletions

diff --git a/usr.sbin/ctld/ctld.cc b/usr.sbin/ctld/ctld.cc
index e3050974dd35..cf1dde93b86d 100644
--- a/usr.sbin/ctld/ctld.cc
+++ b/usr.sbin/ctld/ctld.cc
@@ -59,6 +59,8 @@
 #include "ctld.hh"
 #include "isns.hh"
 
+static freebsd::pidfile pidfile;
+
 bool proxy_mode = false;
 
 static volatile bool sighup_received = false;
@@ -125,17 +127,14 @@ conf::set_pidfile_path(std::string_view path)
 	return (true);
 }
 
-void
-conf::open_pidfile()
+static void
+open_pidfile(const char *path)
 {
-	const char *path;
 	pid_t otherpid;
 
-	assert(!conf_pidfile_path.empty());
-	path = conf_pidfile_path.c_str();
 	log_debugx("opening pidfile %s", path);
-	conf_pidfile = pidfile_open(path, 0600, &otherpid);
-	if (!conf_pidfile) {
+	pidfile = pidfile_open(path, 0600, &otherpid);
+	if (!pidfile) {
 		if (errno == EEXIST)
 			log_errx(1, "daemon already running, pid: %jd.",
 			    (intmax_t)otherpid);
@@ -143,18 +142,6 @@ conf::open_pidfile()
 	}
 }
 
-void
-conf::write_pidfile()
-{
-	conf_pidfile.write();
-}
-
-void
-conf::close_pidfile()
-{
-	conf_pidfile.close();
-}
-
 #ifdef ICL_KERNEL_PROXY
 int
 conf::add_proxy_portal(portal *portal)
@@ -1972,12 +1959,10 @@ conf::apply(struct conf *oldconf)
 	}
 
 	/*
-	 * On startup, oldconf created via conf_new_from_kernel will
-	 * not contain a valid pidfile_path, and the current
-	 * conf_pidfile will already own the pidfile.  On shutdown,
-	 * the temporary newconf will not contain a valid
-	 * pidfile_path, and the pidfile will be cleaned up when the
-	 * oldconf is deleted.
+	 * Rename the pidfile if the pathname changes.  On startup,
+	 * oldconf created via conf_new_from_kernel will not contain a
+	 * valid pidfile_path.  On shutdown, the temporary newconf
+	 * will not contain a valid pidfile_path.
 	 */
 	if (!oldconf->conf_pidfile_path.empty() &&
 	    !conf_pidfile_path.empty()) {
@@ -1992,7 +1977,6 @@ conf::apply(struct conf *oldconf)
 				    conf_pidfile_path.c_str());
 			}
 		}
-		conf_pidfile = std::move(oldconf->conf_pidfile);
 	}
 
 	/*
@@ -2358,7 +2342,7 @@ handle_connection(struct portal *portal, freebsd::fd_up fd,
 			log_err(1, "fork");
 		if (pid > 0)
 			return;
-		conf->close_pidfile();
+		pidfile.close();
 	}
 
 	error = getnameinfo(client_sa, client_sa->sa_len,
@@ -2705,7 +2689,7 @@ main(int argc, char **argv)
 	if (test_config)
 		return (0);
 
-	newconf->open_pidfile();
+	open_pidfile(newconf->pidfile_path());
 
 	register_signals();
 
@@ -2739,7 +2723,7 @@ main(int argc, char **argv)
 
 	oldconf.reset();
 
-	newconf->write_pidfile();
+	pidfile.write();
 
 	newconf->isns_schedule_update();
 
diff --git a/usr.sbin/ctld/ctld.hh b/usr.sbin/ctld/ctld.hh
index a1bd5a62cf3b..61213119f911 100644
--- a/usr.sbin/ctld/ctld.hh
+++ b/usr.sbin/ctld/ctld.hh
@@ -459,6 +459,7 @@ struct conf {
 	int maxproc() const { return conf_maxproc; }
 	int timeout() const { return conf_timeout; }
 	uint32_t genctr() const { return conf_genctr; }
+	const char *pidfile_path() const { return conf_pidfile_path.c_str(); }
 
 	bool default_auth_group_defined() const
 	{ return conf_default_ag_defined; }
@@ -504,10 +505,6 @@ struct conf {
 	bool set_pidfile_path(std::string_view path);
 	void set_timeout(int timeout);
 
-	void open_pidfile();
-	void write_pidfile();
-	void close_pidfile();
-
 	bool add_isns(const char *addr);
 	void isns_register_targets(struct isns *isns, struct conf *oldconf);
 	void isns_deregister_targets(struct isns *isns);
@@ -542,8 +539,6 @@ private:
 	int				conf_maxproc = 30;
 	uint32_t			conf_genctr = 0;
 
-	freebsd::pidfile		conf_pidfile;
-
 	bool				conf_default_pg_defined = false;
 	bool				conf_default_tg_defined = false;
 	bool				conf_default_ag_defined = false;