Merge pull request #1424 from jimklimov/issue-1423

SIGKILL an earlier driver instance if SIGTERM does not cut it [part of #1423 fix]
networkupstools · May 17, 2022 · fa5c719 · fa5c719
2 parents 731fd25 + 31099a7
commit fa5c719
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 6 deletions.
diff --git a/drivers/main.c b/drivers/main.c
@@ -879,17 +879,39 @@ int main(int argc, char **argv)
 				break;
 			}
 
+			upslogx(LOG_WARNING, "Duplicate driver instance detected (PID file %s exists)! Terminating other driver!", buffer);
+
 			if (sendsignalfn(buffer, SIGTERM) != 0) {
 				/* Can't send signal to PID, assume invalid file */
 				break;
 			}
 
-			upslogx(LOG_WARNING, "Duplicate driver instance detected (PID file %s exists)! Terminating other driver!", buffer);
-
 			/* Allow driver some time to quit */
 			sleep(5);
 		}
 
+		if (i > 0) {
+			struct stat	st;
+			if (stat(buffer, &st) == 0) {
+				upslogx(LOG_WARNING, "Duplicate driver instance is still alive (PID file %s exists) after several termination attempts! Killing other driver!", buffer);
+				if (sendsignalfn(buffer, SIGKILL) == 0) {
+					sleep(5);
+					if (sendsignalfn(buffer, 0) == 0) {
+						upslogx(LOG_WARNING, "Duplicate driver instance is still alive (could signal the process)");
+						/* TODO: Should we writepid() below in this case?
+						 * Or if driver init fails, restore the old content
+						 * for that running sibling? */
+					} else {
+						upslogx(LOG_WARNING, "Could not signal the other driver after kill, either its process is finally dead or owned by another user!");
+					}
+				} else {
+					upslogx(LOG_WARNING, "Could not signal the other driver, either its process is dead or owned by another user!");
+				}
+				/* Note: PID file would remain here, but invalid
+				 * as far as further killers would be concerned */
+			}
+		}
+
 		/* Only write pid if we're not just dumping data, for discovery */
 		if (!dump_data) {
 			pidfn = xstrdup(buffer);

diff --git a/drivers/upsdrvctl.c b/drivers/upsdrvctl.c
@@ -146,7 +146,7 @@ void do_upsconf_args(char *upsname, char *var, char *val)
 static void stop_driver(const ups_t *ups)
 {
 	char	pidfn[SMALLBUF];
-	int	ret;
+	int	ret, i;
 	struct stat	fs;
 
 	upsdebugx(1, "Stopping UPS: %s", ups->upsname);
@@ -176,10 +176,40 @@ static void stop_driver(const ups_t *ups)
 	ret = sendsignalfn(pidfn, SIGTERM);
 
 	if (ret < 0) {
-		upslog_with_errno(LOG_ERR, "Stopping %s failed", pidfn);
-		exec_error++;
-		return;
+		upsdebugx(2, "SIGTERM to %s failed, retrying with SIGKILL", pidfn);
+		ret = sendsignalfn(pidfn, SIGKILL);
+		if (ret < 0) {
+			upslog_with_errno(LOG_ERR, "Stopping %s failed", pidfn);
+			exec_error++;
+			return;
+		}
 	}
+
+	for (i = 0; i < 5 ; i++) {
+		if (sendsignalfn(pidfn, 0) != 0) {
+			upsdebugx(2, "Sending signal to %s failed, driver is finally down or wrongly owned", pidfn);
+			return;
+		}
+		sleep(1);
+	}
+
+	upslog_with_errno(LOG_ERR, "Stopping %s failed, retrying harder", pidfn);
+	ret = sendsignalfn(pidfn, SIGKILL);
+	if (ret == 0) {
+		for (i = 0; i < 5 ; i++) {
+			if (sendsignalfn(pidfn, 0) != 0) {
+				upsdebugx(2, "Sending signal to %s failed, driver is finally down or wrongly owned", pidfn);
+				// While a TERMinated driver cleans up,
+				// a stuck and KILLed one does not, so:
+				unlink(pidfn);
+				return;
+			}
+			sleep(1);
+		}
+	}
+
+	upslog_with_errno(LOG_ERR, "Stopping %s failed", pidfn);
+	exec_error++;
 }
 
 static void waitpid_timeout(const int sig)