socket-utils: Ensure child processes are terminated upon socket close

Addresses issue #7.

Added some rudimentary PID to FD tracking so that when we close a file
descriptor opened by `open_serial_socket()`, the process behind that socket
(if any) is explicitly terminated.

This unfortunately requires us to add a new function called
`close_serial_socket()`, but in this particular case the change was not
onerous.

We are only handling five such sockets for now. If there are ever more than
that many system sockets opened through this mechanism, then something is
likely seriously wrong.

The socket tracking code is super simple and possibly not thread safe,
which is fine in this case since these methods will only ever be called
from the same thread.

I really wish these sorts of antics were not necessary, but all other
methods of handling this sort of thing seem to come up short.

configure.ac

@@ -159,7 +159,7 @@ AC_HEADER_TIME
 
 AC_CHECK_HEADERS([unistd.h errno.h stdbool.h], [], [AC_MSG_ERROR(["Missing a required header."])])
 
-AC_CHECK_HEADERS([sys/un.h sys/wait.h pty.h pwd.h execinfo.h asm/sigcontext.h])
+AC_CHECK_HEADERS([sys/un.h sys/wait.h pty.h pwd.h execinfo.h asm/sigcontext.h sys/prctl.h])
 
 AC_C_CONST
 AC_TYPE_SIZE_T

src/util/SuperSocket.cpp

@@ -65,6 +65,11 @@ SuperSocket::SuperSocket(const std::string& path)
 	}
 }
 
+SuperSocket::~SuperSocket()
+{
+	SuperSocket::hibernate();
+}
+
 boost::shared_ptr<SocketWrapper>
 SuperSocket::create(const std::string& path)
 {
@@ -74,14 +79,12 @@ SuperSocket::create(const std::string& path)
 int
 SuperSocket::hibernate(void)
 {
-	syslog(LOG_DEBUG, "SuperSocket::hibernate()");
-
 	if (mFDRead >= 0) {
 		// Unlock the FD.
 		IGNORE_RETURN_VALUE(flock(mFDRead, LOCK_UN));
 
 		// Close the existing FD.
-		IGNORE_RETURN_VALUE(close(mFDRead));
+		IGNORE_RETURN_VALUE(close_serial_socket(mFDRead));
 	}
 
 	mFDRead = -1;
@@ -95,16 +98,13 @@ SuperSocket::reset()
 {
 	syslog(LOG_DEBUG, "SuperSocket::reset()");
 
-	// Unlock the FD.
-	IGNORE_RETURN_VALUE(flock(mFDRead, LOCK_UN));
-
-	// Close the existing FD.
-	IGNORE_RETURN_VALUE(close(mFDRead));
+	SuperSocket::hibernate();
 
 	// Sleep for 200ms to wait for things to settle down.
 	usleep(MSEC_PER_SEC * 200);
 
 	mFDRead = mFDWrite = open_serial_socket(mPath.c_str());
+
 	if (mFDRead < 0) {
 		// Unable to reopen socket...!
 		syslog(LOG_ERR, "SuperSocket::Reset: Unable to reopen socket <%s>, errno=%d (%s)", mPath.c_str(), errno, strerror(errno));

src/util/SuperSocket.h

@@ -33,6 +33,8 @@ class SuperSocket : public UnixSocket {
 	SuperSocket(const std::string& path);
 
 public:
+	virtual ~SuperSocket();
+
 	static boost::shared_ptr<SocketWrapper> create(const std::string& path);
 
 	virtual void reset();

src/util/socket-utils.c

@@ -41,6 +41,7 @@
 #include <netinet/in.h>
 #include <sys/ioctl.h>
 #include <ctype.h>
+#include <signal.h>
 
 #if HAVE_SYS_UN_H
 #include <sys/un.h>
@@ -58,6 +59,11 @@
 #include <util.h>
 #endif
 
+#if HAVE_SYS_PRCTL_H
+#include <sys/prctl.h>
+#endif
+
+
 #if !defined(HAVE_PTSNAME) && __APPLE__
 #define HAVE_PTSNAME 1
 #endif
@@ -72,6 +78,102 @@
 
 #include <poll.h>
 
+static struct {
+	int fd;
+	pid_t pid;
+} gSystemSocketTable[5];
+
+static void
+system_socket_table_close_alarm_(int sig)
+{
+	syslog(LOG_ERR, "Unable to terminate child!");
+	exit(EXIT_FAILURE);
+}
+
+static void
+system_socket_table_atexit_(void)
+{
+	int i;
+
+	for (i = 0; i < sizeof(gSystemSocketTable)/sizeof(gSystemSocketTable[0]); i++) {
+		if (gSystemSocketTable[i].pid != 0) {
+			kill(gSystemSocketTable[i].pid, SIGTERM);
+		}
+	}
+}
+
+static void
+system_socket_table_add_(int fd, pid_t pid)
+{
+	static bool did_init = false;
+	int i;
+
+	if (!did_init) {
+		atexit(&system_socket_table_atexit_);
+		did_init = true;
+	}
+
+	for (i = 0; i < sizeof(gSystemSocketTable)/sizeof(gSystemSocketTable[0]); i++) {
+		if (gSystemSocketTable[i].pid == 0) {
+			break;
+		}
+	}
+
+	// If we have more than 5 of these types of sockets
+	// open, then there is likely a serious problem going
+	// on that we should immediately deal with.
+	assert(i < sizeof(gSystemSocketTable)/sizeof(gSystemSocketTable[0]));
+
+	if (i < sizeof(gSystemSocketTable)/sizeof(gSystemSocketTable[0])) {
+		gSystemSocketTable[i].fd = fd;
+		gSystemSocketTable[i].pid = pid;
+	}
+}
+
+int
+close_serial_socket(int fd)
+{
+	int i;
+	int ret;
+	for (i = 0; i < sizeof(gSystemSocketTable)/sizeof(gSystemSocketTable[0]); i++) {
+		if ( gSystemSocketTable[i].fd == fd
+		  && gSystemSocketTable[i].pid != 0
+		) {
+			break;
+		}
+	}
+
+	ret = close(fd);
+
+	if (i < sizeof(gSystemSocketTable)/sizeof(gSystemSocketTable[0])) {
+		// Set up a watchdog timer in case things go sour.
+		void (*prev_alarm_handler)(int) = signal(SIGALRM, &system_socket_table_close_alarm_);
+		unsigned int prev_alarm_remaining = alarm(5);
+		pid_t pid = gSystemSocketTable[i].pid;
+		int x = 0;
+
+		kill(pid, SIGTERM);
+
+		do {
+			errno = 0;
+			pid = waitpid(gSystemSocketTable[i].pid, &x, 0);
+		} while ((pid == -1) && (errno == EINTR));
+
+		if (pid == -1) {
+			perror(strerror(errno));
+			syslog(LOG_ERR, "Failed waiting for death of PID %d: %s", gSystemSocketTable[i].pid, strerror(errno));
+		}
+
+		alarm(prev_alarm_remaining);
+		signal(SIGALRM, prev_alarm_handler);
+
+		gSystemSocketTable[i].pid = 0;
+		gSystemSocketTable[i].fd = -1;
+	}
+
+	return ret;
+}
+
 int
 fd_has_error(int fd)
 {
@@ -117,8 +219,9 @@ socket_name_is_port(const char* socket_name)
 {
 	// It's a port if the string is just a number.
 	while (*socket_name) {
-		if (!isdigit(*socket_name++))
+		if (!isdigit(*socket_name++)) {
 			return false;
+		}
 	}
 
 	return true;
@@ -157,11 +260,13 @@ lookup_sockaddr_from_host_and_port(
 	struct addrinfo *results = NULL;
 	struct addrinfo *iter = NULL;
 
-	if (!port)
+	if (!port) {
 		port = "4951";
+	}
 
-	if (!host)
+	if (!host) {
 		host = "::1";
+	}
 
 	syslog(LOG_INFO, "Looking up [%s]:%s", host, port);
 
@@ -303,6 +408,10 @@ open_system_socket_forkpty(const char* command)
 		const int dtablesize = getdtablesize();
 		int i;
 
+#if defined(_LINUX_PRCTL_H)
+		prctl(PR_SET_PDEATHSIG, SIGHUP);
+#endif
+
 		// Re-instate our original stderr.
 		dup2(stderr_copy_fd, STDERR_FILENO);
 
@@ -336,6 +445,8 @@ open_system_socket_forkpty(const char* command)
 	close(open(ptsname(ret_fd), O_RDWR | O_NOCTTY));
 #endif
 
+	system_socket_table_add_(ret_fd, pid);
+
 	return ret_fd;
 
 cleanup_and_fail:
@@ -376,6 +487,10 @@ fork_unixdomain_socket(int* fd_pointer)
 		const int dtablesize = getdtablesize();
 		// We are the forked process.
 
+#if defined(_LINUX_PRCTL_H)
+		prctl(PR_SET_PDEATHSIG, SIGHUP);
+#endif
+
 		close(fd[0]);
 
 		dup2(fd[1], STDIN_FILENO);
@@ -401,10 +516,10 @@ fork_unixdomain_socket(int* fd_pointer)
 
 	{
 		int prevErrno = errno;
-		if (fd[0] < 0) {
+		if (fd[0] >= 0) {
 			close(fd[0]);
 		}
-		if (fd[1] < 0) {
+		if (fd[1] >= 0) {
 			close(fd[1]);
 		}
 		errno = prevErrno;
@@ -417,7 +532,6 @@ static int
 open_system_socket_unix_domain(const char* command)
 {
 	int fd = -1;
-	int status = 0;
 	pid_t pid = -1;
 
 	pid = fork_unixdomain_socket(&fd);
@@ -429,47 +543,19 @@ open_system_socket_unix_domain(const char* command)
 
 	// Check to see if we are the forked process or not.
 	if (0 == pid) {
-		// Double fork to avoid leaking zombie processes.
-		pid = fork();
-		if (pid < 0) {
-			syslog(LOG_ERR, "Call to fork() failed: %s (%d)", strerror(errno), errno);
-
-			_exit(errno);
-		}
-
-		if (0 == pid)
-		{
-			// Set the shell environment variable if it isn't set already.
-			setenv("SHELL","/bin/sh",0);
-
-			syslog(LOG_NOTICE, "About to exec \"%s\"", command);
-
-			execl(getenv("SHELL"), getenv("SHELL"),"-c", command, NULL);
+		// Set the shell environment variable if it isn't set already.
+		setenv("SHELL","/bin/sh",0);
 
-			syslog(LOG_ERR, "Failed for fork and exec of \"%s\": %s (%d)", command, strerror(errno), errno);
+		syslog(LOG_NOTICE, "About to exec \"%s\"", command);
 
-			_exit(EXIT_FAILURE);
-		}
+		execl(getenv("SHELL"), getenv("SHELL"),"-c", command, NULL);
 
-		_exit(EXIT_SUCCESS);
-	}
+		syslog(LOG_ERR, "Failed for fork and exec of \"%s\": %s (%d)", command, strerror(errno), errno);
 
-	// Wait for the first fork to return, and place the return value in errno
-	if (waitpid(pid, &status, 0) < 0) {
-		syslog(LOG_ERR, "Call to waitpid() failed: %s (%d)", strerror(errno), errno);
+		_exit(EXIT_FAILURE);
 	}
 
-	if (0 != WEXITSTATUS(status)) {
-		// If this has happened then the double fork failed. Clean up
-		// and pass this status along to the caller as errno.
-
-		syslog(LOG_ERR, "Child process failed: %s (%d)", strerror(WEXITSTATUS(status)), WEXITSTATUS(status));
-
-		close(fd);
-		fd = -1;
-
-		errno = WEXITSTATUS(status);
-	}
+	system_socket_table_add_(fd, pid);
 
 	return fd;
 
@@ -665,12 +751,10 @@ open_serial_socket(const char* socket_name)
 #endif
 
 #define FETCH_TERMIOS() do { if(0 != tcgetattr(fd, &tios)) { \
-			/* perror("tcgetattr"); */ \
 			syslog(LOG_DEBUG, "tcgetattr() failed. \"%s\" (%d)", strerror(errno), errno); \
 		} } while(0)
 
 #define COMMIT_TERMIOS() do { if(0 != tcsetattr(fd, TCSANOW, &tios)) { \
-			/* perror("tcsetattr"); */ \
 			syslog(LOG_DEBUG, "tcsetattr() failed. \"%s\" (%d)", strerror(errno), errno); \
 		} } while(0)
 

src/util/socket-utils.h

@@ -41,6 +41,7 @@ bool socket_name_is_inet(const char* socket_name);
 bool socket_name_is_device(const char* socket_name);
 int lookup_sockaddr_from_host_and_port( struct sockaddr_in6* outaddr, const char* host, const char* port);
 int open_serial_socket(const char* socket_name);
+int close_serial_socket(int fd);
 int fd_has_error(int fd);
 
 int fork_unixdomain_socket(int* fd_pointer);