Overhaul kill code

Engines were not always forcefully killed in XBoard, which could make
the tournament manager hang when an engine became unresponsive to "quit".
The various levels of killing are now revised, and the -delayAfterQuit
is absorbed in DestroyChildProcess(). There is a hard killing mode 9,
which immediately sends SIGKILL, intended for engines that have already
proven to malfunction or could not start at all. In other cases SIGTERM
is sent according to specs, but a SIGKILL is scheduled -delayAfterQuit + 1
sec later just in case the SIGTERM would be ignored.

diff --git a/backend.c b/backend.c
index ab8d1d5..ab34737 100644 (file)
--- a/backend.c
+++ b/backend.c
@@ -777,8 +777,7 @@ UnloadEngine (ChessProgramState *cps)
            ExitAnalyzeMode();
             DoSleep( appData.delayBeforeQuit );
            SendToProgram("quit\n", cps);
-            DoSleep( appData.delayAfterQuit );
-           DestroyChildProcess(cps->pr, cps->useSigterm);
+           DestroyChildProcess(cps->pr, 4 + cps->useSigterm);
        }
        cps->pr = NoProc;
        if(appData.debugMode) fprintf(debugFP, "Unload %s\n", cps->which);
@@ -11352,8 +11351,7 @@ GameEnds (ChessMove result, char *resultDetails, int whosays)
            ExitAnalyzeMode();
             DoSleep( appData.delayBeforeQuit );
            SendToProgram("quit\n", &first);
-            DoSleep( appData.delayAfterQuit );
-           DestroyChildProcess(first.pr, first.useSigterm);
+           DestroyChildProcess(first.pr, 4 + first.useSigterm);
            first.reload = TRUE;
        }
        first.pr = NoProc;
@@ -11378,8 +11376,7 @@ GameEnds (ChessMove result, char *resultDetails, int whosays)
        if (second.pr != NoProc) {
             DoSleep( appData.delayBeforeQuit );
            SendToProgram("quit\n", &second);
-            DoSleep( appData.delayAfterQuit );
-           DestroyChildProcess(second.pr, second.useSigterm);
+           DestroyChildProcess(second.pr, 4 + second.useSigterm);
            second.reload = TRUE;
        }
        second.pr = NoProc;
@@ -13975,14 +13972,12 @@ ExitEvent (int status)
 
         DoSleep( appData.delayBeforeQuit );
        SendToProgram("quit\n", &first);
-        DoSleep( appData.delayAfterQuit );
-       DestroyChildProcess(first.pr, 10 /* [AS] first.useSigterm */ );
+       DestroyChildProcess(first.pr, 4 + first.useSigterm /* [AS] first.useSigterm */ );
     }
     if (second.pr != NoProc) {
         DoSleep( appData.delayBeforeQuit );
        SendToProgram("quit\n", &second);
-        DoSleep( appData.delayAfterQuit );
-       DestroyChildProcess(second.pr, 10 /* [AS] second.useSigterm */ );
+       DestroyChildProcess(second.pr, 4 + second.useSigterm /* [AS] second.useSigterm */ );
     }
     if (first.isr != NULL) {
        RemoveInputSource(first.isr);

diff --git a/usystem.c b/usystem.c
index 2334572..6ac238c 100644 (file)
--- a/usystem.c
+++ b/usystem.c
@@ -497,9 +497,12 @@ StartChildProcess (char *cmdLine, char *dir, ProcRef *pr)
 }
 
 // [HGM] kill: implement the 'hard killing' of AS's Winboard_x
+static int pid;
+
 static RETSIGTYPE
 AlarmCallBack (int n)
 {
+    kill(pid, SIGKILL); // kill forcefully
     return;
 }
 
@@ -510,22 +513,17 @@ DestroyChildProcess (ProcRef pr, int signalType)
 
     if (cp->kind != CPReal) return;
     cp->kind = CPNone;
-    if (signalType == 10) { // [HGM] kill: if it does not terminate in 3 sec, kill
-       signal(SIGALRM, AlarmCallBack);
-       alarm(3);
-       if(wait((int *) 0) == -1) { // process does not terminate on its own accord
-           kill(cp->pid, SIGKILL); // kill it forcefully
-           wait((int *) 0);        // and wait again
-       }
-    } else {
-       if (signalType) {
-           kill(cp->pid, signalType == 9 ? SIGKILL : SIGTERM); // [HGM] kill: use hard kill if so requested
-       }
-       /* Process is exiting either because of the kill or because of
-          a quit command sent by the backend; either way, wait for it to die.
-       */
-       wait((int *) 0);
+    if (signalType & 1) {
+           kill(cp->pid, signalType == 9 ? SIGKILL : SIGTERM); // [HGM] kill: for 9 hard-kill immediately
     }
+    signal(SIGALRM, AlarmCallBack);
+    pid = cp->pid;
+    if(signalType & 4) alarm(1 + appData.delayAfterQuit); // [HGM] kill: schedule hard kill if so requested
+    /* Process is exiting either because of the kill or because of
+       a quit command sent by the backend; either way, wait for it to die.
+    */
+    wait((int *) 0);
+    alarm(0); // cancel alarm if still pending
     close(cp->fdFrom);
     close(cp->fdTo);
 }

diff --git a/winboard/winboard.c b/winboard/winboard.c
index 1b8460b..d83aaaa 100644 (file)
--- a/winboard/winboard.c
+++ b/winboard/winboard.c
@@ -9234,15 +9234,15 @@ DestroyChildProcess(ProcRef pr, int/*boolean*/ signal)
     /*!!if (signal) GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, cp->pid);*/\r
 \r
     /* [AS] Special termination modes for misbehaving programs... */\r
-    if( signal == 9 ) { \r
+    if( signal & 8 ) { \r
         result = TerminateProcess( cp->hProcess, 0 );\r
 \r
         if ( appData.debugMode) {\r
             fprintf( debugFP, "Terminating process %lu, result=%d\n", cp->pid, result );\r
         }\r
     }\r
-    else if( signal == 10 ) {\r
-        DWORD dw = WaitForSingleObject( cp->hProcess, 3*1000 ); // Wait 3 seconds at most\r
+    else if( signal & 4 ) {\r
+        DWORD dw = WaitForSingleObject( cp->hProcess, appData.delayAfterQuit*1000 + 50 ); // Wait 3 seconds at most\r
 \r
         if( dw != WAIT_OBJECT_0 ) {\r
             result = TerminateProcess( cp->hProcess, 0 );\r

diff --git a/xboard.texi b/xboard.texi
index d631405..9e61977 100644 (file)
--- a/xboard.texi
+++ b/xboard.texi
@@ -3563,7 +3563,14 @@ Default is the login name on your local computer.
 @itemx -delayAfterQuit number
 @cindex delayBeforeQuit, option
 @cindex delayAfterQuit, option
-These options specify how long XBoard has to wait before sending a termination signal to rogue engine processes, that do not want to react to the 'quit' command. The second one determines the pause after killing the engine, to make sure it dies.
+These options order pauses before and after sending the "quit" command to an engine that must be terminated.
+The pause between quit and the previous command is specified in milliseconds.
+The pause after quit is used to schedule a kill signal to be sent to the engine process after the
+number of specified seconds plus one.
+This signal is a different one as the terminiation signal described in the protocol specs
+which engines can suppress or ignore, and which is sent directly after the "quit" command.
+Setting @code{delayAfterQuit} to -1 will suppress sending of the kill signal.
+Default: 0
 @item -searchMode n
 @cindex searchMode, option
 The integer n encodes the mode for the @samp{find position} function.