fix(poller): isolate worker failures and bound database reconnects

Makefile.am

@@ -65,7 +65,9 @@ test_spine_faults_SOURCES = $(spine_shared_sources) tests/test_spine_faults.c
 test_spine_faults_CPPFLAGS = $(AM_CPPFLAGS) -I$(srcdir)
 test_spine_faults_LDADD = test_spine_regressions-spine.$(OBJEXT)
 test_spine_faults_LDFLAGS = $(AM_LDFLAGS) -Wl,--wrap=strftime -Wl,--wrap=mysql_query -Wl,--wrap=mysql_ping -Wl,--wrap=mysql_errno \
- -Wl,--wrap=pipe -Wl,--wrap=socketpair -Wl,--wrap=posix_spawn \
+ -Wl,--wrap=mysql_real_connect -Wl,--wrap=mysql_thread_id -Wl,--wrap=mysql_store_result \
+ -Wl,--wrap=mysql_num_rows -Wl,--wrap=mysql_fetch_row -Wl,--wrap=mysql_free_result -Wl,--wrap=mysql_num_fields \
+ -Wl,--wrap=pipe -Wl,--wrap=socketpair -Wl,--wrap=posix_spawn -Wl,--wrap=posix_spawnattr_setpgroup \
  -Wl,--wrap=malloc -Wl,--wrap=strdup \
  -Wl,--wrap=snmp_sess_open -Wl,--wrap=snmp_sess_close -Wl,--wrap=pthread_create \
  -Wl,--wrap=getuid -Wl,--wrap=geteuid -Wl,--wrap=getgid -Wl,--wrap=getegid \

Makefile.in

@@ -836,7 +836,9 @@ test_spine_regressions_CPPFLAGS = $(AM_CPPFLAGS) -I$(srcdir) -DSPINE_TEST_PROGRA
 @HAVE_LINKER_WRAP_TRUE@test_spine_faults_CPPFLAGS = $(AM_CPPFLAGS) -I$(srcdir)
 @HAVE_LINKER_WRAP_TRUE@test_spine_faults_LDADD = test_spine_regressions-spine.$(OBJEXT)
 @HAVE_LINKER_WRAP_TRUE@test_spine_faults_LDFLAGS = $(AM_LDFLAGS) -Wl,--wrap=strftime -Wl,--wrap=mysql_query -Wl,--wrap=mysql_ping -Wl,--wrap=mysql_errno \
-@HAVE_LINKER_WRAP_TRUE@ -Wl,--wrap=pipe -Wl,--wrap=socketpair -Wl,--wrap=posix_spawn \
+@HAVE_LINKER_WRAP_TRUE@ -Wl,--wrap=mysql_real_connect -Wl,--wrap=mysql_thread_id -Wl,--wrap=mysql_store_result \
+@HAVE_LINKER_WRAP_TRUE@ -Wl,--wrap=mysql_num_rows -Wl,--wrap=mysql_fetch_row -Wl,--wrap=mysql_free_result -Wl,--wrap=mysql_num_fields \
+@HAVE_LINKER_WRAP_TRUE@ -Wl,--wrap=pipe -Wl,--wrap=socketpair -Wl,--wrap=posix_spawn -Wl,--wrap=posix_spawnattr_setpgroup \
 @HAVE_LINKER_WRAP_TRUE@ -Wl,--wrap=malloc -Wl,--wrap=strdup \
 @HAVE_LINKER_WRAP_TRUE@ -Wl,--wrap=snmp_sess_open -Wl,--wrap=snmp_sess_close -Wl,--wrap=pthread_create \
 @HAVE_LINKER_WRAP_TRUE@ -Wl,--wrap=getuid -Wl,--wrap=geteuid -Wl,--wrap=getgid -Wl,--wrap=getegid \

error.c

@@ -98,12 +98,15 @@ static void spine_signal_handler(int spine_signal) {
 	const char *message = NULL;
 	int saved_errno = errno;
 
-	/* A real fault (SIGSEGV/SIGBUS/...) resets to SIG_DFL and returns so the
-	 * faulting instruction re-executes and dies with a core. SIGPIPE is a
-	 * routine, recurring condition instead, and signal dispositions are
-	 * process-wide: dropping it to SIG_DFL here, even briefly, would let a
-	 * broken-pipe write on any other thread terminate the process during
-	 * that window. Leave this handler permanently installed for SIGPIPE. */
+	/* Every signal handled here except SIGSEGV, which exits with status 1
+	 * below, resets to SIG_DFL and is raised again before returning, so the
+	 * process ends with that signal's default action and core. Returning alone only ended real faults, which re-execute; after
+	 * SIGINT, SIGQUIT or a sent SIGBUS the poller logged FATAL and kept going.
+	 * SIGPIPE is a routine, recurring condition instead, and signal
+	 * dispositions are process-wide: dropping it to SIG_DFL here, even
+	 * briefly, would let a broken-pipe write on any other thread terminate
+	 * the process during that window. Leave this handler permanently
+	 * installed for SIGPIPE. */
 	if (spine_signal != SIGPIPE) {
 		signal(spine_signal, SIG_DFL);
 	}
@@ -149,6 +152,12 @@ static void spine_signal_handler(int spine_signal) {
 		_exit(1);
 	}
 
+	/* The signal stays blocked until this handler returns, then the default
+	 * action ends the process. raise() is async-signal-safe. */
+	if (spine_signal != SIGPIPE) {
+		(void) raise(spine_signal);
+	}
+
 	errno = saved_errno;
 }
 

nft_popen.c

@@ -364,6 +364,9 @@ int nft_popen(const char * command, const char * type) {
 	/* nft_popen() reports why process creation failed through errno, as
 	 * popen() does; cleanup below must not overwrite it. */
 	int    failure_errno = 0;
+	posix_spawnattr_t attr;
+	int    attr_valid = FALSE;
+	int    attr_err;
 
 	if (command == NULL || type == NULL) {
 		errno = EINVAL;
@@ -445,6 +448,21 @@ int nft_popen(const char * command, const char * type) {
 		return -1;
 	}
 
+	/* Each script leads its own process group so a timeout can kill the
+	 * group. Killing only the shell left anything it had started running,
+	 * still holding the pipe and counting against the process limit. */
+	attr_err = posix_spawnattr_init(&attr);
+	if (attr_err == 0) {
+		attr_valid = TRUE;
+		attr_err = posix_spawnattr_setpgroup(&attr, 0);
+	}
+	if (attr_err == 0) attr_err = posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETPGROUP);
+	if (attr_err != 0) {
+		failure_errno = attr_err;
+		SPINE_LOG(("ERROR: SCRIPT: posix_spawnattr setup failed: %s", strerror(attr_err)));
+		goto spawn_failed;
+	}
+
 	/* The pipe ends are close-on-exec, which is the point: another thread
 	 * spawning in this window must not inherit them. The child needs its own
 	 * end, and dup2 clears the flag on its target, so the usual paths are
@@ -521,7 +539,7 @@ int nft_popen(const char * command, const char * type) {
 
 	int spawn_err;
 	retry:
-	spawn_err = posix_spawn(&pid, spawn_shell, &fa, NULL, argv, environ);
+	spawn_err = posix_spawn(&pid, spawn_shell, &fa, &attr, argv, environ);
 
 	if (spawn_err != 0) {
 		if ((spawn_err == EAGAIN || spawn_err == ENOMEM) && retry_count < 3) {
@@ -540,6 +558,7 @@ int nft_popen(const char * command, const char * type) {
 		 * nft_pclose() in every poller thread and the daemon stops collecting
 		 * script data until it is restarted. */
 		posix_spawn_file_actions_destroy(&fa);
+		if (attr_valid) posix_spawnattr_destroy(&attr);
 
 		if (inherit_fd != -1) {
 			(void)close(inherit_fd);
@@ -559,6 +578,7 @@ int nft_popen(const char * command, const char * type) {
 	}
 
 	posix_spawn_file_actions_destroy(&fa);
+	posix_spawnattr_destroy(&attr);
 
 	/* The child holds its own duplicate. Keeping this one would hold the pipe's
 	 * write end open, so the reader never sees EOF and exec_poll() blocks to
@@ -688,7 +708,8 @@ nft_pclose(int fd)
 		pid = cur->pid;
 		break;
 	case 1:
-		(void)kill(cur->pid, SIGKILL);
+		/* The negative pid reaches the script's whole process group. */
+		(void)kill(-cur->pid, SIGKILL);
 		nft_abandon_child(cur->pid, "did not exit before pipe close");
 		errno = ETIMEDOUT;
 		pid = -1;
@@ -699,7 +720,7 @@ nft_pclose(int fd)
 		 * outside the sweep's reach. Preserve waitpid()'s errno. */
 		{
 			int saved_errno = errno;
-			(void)kill(cur->pid, SIGKILL);
+			(void)kill(-cur->pid, SIGKILL);
 			nft_abandon_child(cur->pid, "waitpid failed");
 			errno = saved_errno;
 		}

php.c

@@ -84,6 +84,17 @@ static char *php_undefined_result(void) {
 	return result;
 }
 
+/* Script and script server arguments can carry SNMP communities and v3
+ * passphrases, so logs name only the first token, the script, never the
+ * arguments. */
+void php_command_script(const char *command, char *script, size_t capacity) {
+	size_t length = strcspn(command, " \t\r\n");
+
+	if (length >= capacity) length = capacity - 1;
+	memcpy(script, command, length);
+	script[length] = '\0';
+}
+
 static void php_fail_read(int php_process, int allow_restart) {
 	php_processes[php_process].php_state = PHP_BUSY;
 	if (allow_restart) {
@@ -106,7 +117,7 @@ void php_processes_initialize(php_t *processes, int count) {
 	}
 }
 
-static char *php_read_result(int php_process, char *command, int allow_restart);
+static char *php_read_result(int php_process, const char *command, int allow_restart);
 
 /* Block SIGPIPE in the calling thread around Spine's two pipe writes. The
  * daemon normally catches SIGPIPE with a no-op handler process-wide, but this local guard
@@ -201,7 +212,11 @@ char *php_cmd(const char *php_command, int php_process) {
 
 	/* if write status is <= 0 then the script server may be hung */
 	if (bytes <= 0) {
-		SPINE_LOG(("ERROR: SS[%i] PHP Script Server communications lost sending Command[%s].  Restarting PHP Script Server", php_process, command));
+		char script[SMALL_BUFSIZE];
+
+		php_command_script(command, script, sizeof(script));
+		SPINE_LOG(("ERROR: SS[%i] PHP Script Server communications lost sending a command.  Restarting PHP Script Server", php_process));
+		SPINE_LOG_DEBUG(("DEBUG: SS[%i] Unsent command for script '%s'", php_process, script));
 
 		php_close(php_process);
 		retries++;
@@ -323,7 +338,7 @@ char *php_read_result_for_test(int php_process, char *command, int allow_restart
 }
 #endif
 
-/*! \fn char *php_readpipe(int php_process, char *command)
+/*! \fn char *php_readpipe(int php_process, const char *command)
  *  \brief read a line from a PHP Script Server process
  *  \param php_process the PHP Script Server process to obtain output from
  *
@@ -334,7 +349,7 @@ char *php_read_result_for_test(int php_process, char *command, int allow_restart
  *
  *  \return a string pointer to the PHP Script Server response
  */
-/*! \fn static char *php_read_result(int php_process, char *command, int allow_restart)
+/*! \fn static char *php_read_result(int php_process, const char *command, int allow_restart)
  *  \brief reads one script server response.
  *
  *  allow_restart is FALSE for the startup handshake. php_init() calls this to
@@ -344,7 +359,7 @@ char *php_read_result_for_test(int php_process, char *command, int allow_restart
  *  recursion, spawning a fresh server at every level. Refusing the restart on
  *  the handshake bounds the depth at one by construction.
  */
-static char *php_read_result(int php_process, char *command, int allow_restart) {
+static char *php_read_result(int php_process, const char *command, int allow_restart) {
 	fd_set fds;
 	struct timeval timeout;
 	double begin_time = 0;
@@ -353,6 +368,7 @@ static char *php_read_result(int php_process, char *command, int allow_restart)
 	char *result_string;
 	int response_timeout;
 	double read_deadline;
+	char script[SMALL_BUFSIZE];
 
 	ssize_t i;
 	char *cp;
@@ -450,7 +466,9 @@ static char *php_read_result(int php_process, char *command, int allow_restart)
 	case 0:
 		/* record end time */
 		end_time = get_time_as_double();
-		SPINE_LOG(("WARNING: SS[%i] The PHP Script Server did not respond in time for Timeout[%0.2f], Command[%s] and will therefore be restarted", php_process, end_time - begin_time, command));
+		SPINE_LOG(("WARNING: SS[%i] The PHP Script Server did not respond in time for Timeout[%0.2f] and will therefore be restarted", php_process, end_time - begin_time));
+		php_command_script(command, script, sizeof(script));
+		SPINE_LOG_DEBUG(("DEBUG: SS[%i] Unanswered command for script '%s'", php_process, script));
 		SET_UNDEFINED(result_string);
 		php_fail_read(php_process, allow_restart);
 		break;
@@ -478,7 +496,9 @@ static char *php_read_result(int php_process, char *command, int allow_restart)
 						int ready = spine_wait_readable(php_processes[php_process].php_read_fd, read_deadline);
 
 						if (ready <= 0) {
-							SPINE_LOG(("WARNING: SS[%i] The PHP Script Server sent a partial response and %s, Command[%s]", php_process, ready == 0 ? "timed out" : "failed", command));
+							SPINE_LOG(("WARNING: SS[%i] The PHP Script Server sent a partial response and %s", php_process, ready == 0 ? "timed out" : "failed"));
+							php_command_script(command, script, sizeof(script));
+							SPINE_LOG_DEBUG(("DEBUG: SS[%i] Unanswered command for script '%s'", php_process, script));
 							SET_UNDEFINED(result_string);
 							read_ok = FALSE;
 							break;
@@ -529,13 +549,13 @@ static char *php_read_result(int php_process, char *command, int allow_restart)
 	return result_string;
 }
 
-/*! \fn char *php_readpipe(int php_process, char *command)
+/*! \fn char *php_readpipe(int php_process, const char *command)
  *  \brief reads a script server response, restarting a server that stops
  *         answering.
  *
  *  \return a string pointer to the PHP Script Server response
  */
-char *php_readpipe(int php_process, char *command) {
+char *php_readpipe(int php_process, const char *command) {
 	return php_read_result(php_process, command, TRUE);
 }
 

php.h

@@ -34,7 +34,8 @@
 #ifndef SPINE_PHP_H
 #define SPINE_PHP_H
 extern char *php_cmd(const char *php_command, int php_process);
-extern char *php_readpipe(int php_process, char *command);
+extern char *php_readpipe(int php_process, const char *command);
+extern void php_command_script(const char *command, char *script, size_t capacity);
 extern int php_init(int php_process);
 extern void php_close(int php_process);
 extern int php_get_process(void);