From e7109c87bc43f21636c5f9814af55d7ff11899ae Mon Sep 17 00:00:00 2001
From: Rob Swindell <rob@synchro.net>
Date: Fri, 7 Oct 2022 18:31:59 -0700
Subject: [PATCH] When user hangs-up on external programs on *nix, try to
 terminate w/SIGTERM

Previously, when a user disconnected or ran out of time while running a
stdio-based external program on *nix, if the program was still running, we'd
send it a SIGHUP, wait up to 10 seconds for the process to terminate and if
it did not, terminate it (ungracefully) with SIGKILL. Since some programs
catch SIGTERM (and not SIGHUP) to indicate a termination request, we now will
first attempt a SIGHUP, wait up to 5 seconds for the process to terminate and
if it does not, then send a SIGTERM and wait up to another 5 seconds for it
to terminate and if it doesn't, then finally send it a SIGKILL (which cannot
be caught and always results in an ungraceful termination of the child
process).

This doesn't resolve any specific problem with any specific stdio-based
external program, but I was playing around with ESR's port of Adventure
(https://gitlab.com/esr/open-adventure) and a new auto-save/restore of game
state and noticed that we weren't using SIGTERM for this situation, though we
should have. Most modern programs, if they catch SIGHUP at all, use it to
indicate a refresh of configuration or data files, not a termination request
(or indication that a user has "hung up"). So SIGTERM is more reasonable to be
expected to be caught and initiate the graceful termination of the child
program that we're hoping for.
---
 src/sbbs3/xtrn.cpp | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/sbbs3/xtrn.cpp b/src/sbbs3/xtrn.cpp
index 3a581519a7..70f651cdb8 100644
--- a/src/sbbs3/xtrn.cpp
+++ b/src/sbbs3/xtrn.cpp
@@ -1847,14 +1847,23 @@ int sbbs_t::external(const char* cmdline, long mode, const char* startup_dir)
 
 		if(waitpid(pid, &i, WNOHANG)==0)  {		// Child still running?
 			kill(pid, SIGHUP);					// Tell child user has hung up
-			time_t start=time(NULL);			// Wait up to 10 seconds
-			while(time(NULL)-start<10) {		// for child to terminate
+			time_t start=time(NULL);			// Wait up to 5 seconds
+			while(time(NULL)-start<5 ) {		// for child to terminate
 				if(waitpid(pid, &i, WNOHANG)!=0)
 					break;
 				mswait(500);
 			}
-			if(waitpid(pid, &i, WNOHANG)==0)	// Child still running?
-				kill(pid, SIGKILL);				// terminate child process
+			if(waitpid(pid, &i, WNOHANG)==0) {	// Child still running?
+				kill(pid, SIGTERM);				// terminate child process (gracefully)
+				start=time(NULL);				// Wait up to 5 (more) seconds
+				while(time(NULL)-start<5 ) {	// for child to terminate
+					if(waitpid(pid, &i, WNOHANG)!=0)
+						break;
+					mswait(500);
+				}
+				if(waitpid(pid, &i, WNOHANG)==0)// Child still running?
+					kill(pid, SIGKILL);			// terminate child process (ungracefully)
+			}
 		}
 		/* close unneeded descriptors */
 		if(mode&EX_STDIN)
-- 
GitLab