You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ma...@apache.org on 2022/11/23 19:22:15 UTC

[commons-daemon] branch master updated: Follow up for https://issues.redhat.com/browse/JBCS-1261 make sure to kill child processes recursively don't wait forever if there is a timeout set.

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-daemon.git


The following commit(s) were added to refs/heads/master by this push:
     new 243a852  Follow up for https://issues.redhat.com/browse/JBCS-1261 make sure to kill child processes recursively don't wait forever if there is a timeout set.
243a852 is described below

commit 243a8521f780b98247820ae187e395597f6c9b51
Author: Jean-Frederic Clere <jf...@gmail.com>
AuthorDate: Mon Oct 24 17:16:02 2022 +0200

    Follow up for https://issues.redhat.com/browse/JBCS-1261
    make sure to kill child processes recursively
    don't wait forever if there is a timeout set.
---
 src/native/windows/apps/prunsrv/prunsrv.c |  17 +++-
 src/native/windows/include/rprocess.h     |   2 +
 src/native/windows/src/rprocess.c         | 126 ++++++++++++++++++++++--------
 3 files changed, 111 insertions(+), 34 deletions(-)

diff --git a/src/native/windows/apps/prunsrv/prunsrv.c b/src/native/windows/apps/prunsrv/prunsrv.c
index 30db33a..17bdc3c 100644
--- a/src/native/windows/apps/prunsrv/prunsrv.c
+++ b/src/native/windows/apps/prunsrv/prunsrv.c
@@ -229,6 +229,8 @@ static BOOL                  _jni_shutdown = FALSE;
 static BOOL                  _java_startup  = FALSE;
 /* Java used for shutdown    */
 static BOOL                  _java_shutdown = FALSE;
+/* We have request to shutdown the exe running */
+static BOOL                  _exe_shutdown = FALSE;
 /* Global variables and objects */
 static APXHANDLE    gPool;
 static APXHANDLE    gWorker;
@@ -1606,6 +1608,7 @@ void WINAPI service_ctrl_handler(DWORD dwCtrlCode)
             apxLogWrite(APXLOG_MARK_INFO "Service SHUTDOWN signalled.");
         case SERVICE_CONTROL_STOP:
             apxLogWrite(APXLOG_MARK_INFO "Service SERVICE_CONTROL_STOP signalled.");
+            _exe_shutdown = TRUE;
             if (SO_STOPTIMEOUT > 0) {
                 reportServiceStatus(SERVICE_STOP_PENDING, NO_ERROR, SO_STOPTIMEOUT * 1000);
             }
@@ -1834,7 +1837,17 @@ void WINAPI serviceMain(DWORD argc, LPTSTR *argv)
         /* Set console handler to capture CTRL events */
         SetConsoleCtrlHandler((PHANDLER_ROUTINE)console_handler, TRUE);
 
-        apxHandleWait(gWorker, INFINITE, FALSE);
+        if (SO_STOPTIMEOUT) {
+            /* we have a stop timeout */
+	    do {
+                /* wait 2 seconds */
+                apxHandleWait(gWorker, 2000, FALSE);
+            } while (!_exe_shutdown);
+            apxLogWrite(APXLOG_MARK_DEBUG "waiting %d sec... shutdown: %d", SO_STOPTIMEOUT, _exe_shutdown);
+            apxHandleWait(gWorker, SO_STOPTIMEOUT*1000, FALSE);
+	} else {
+             apxHandleWait(gWorker, INFINITE, FALSE);
+	}
         apxLogWrite(APXLOG_MARK_DEBUG "Worker finished.");
     }
     else {
@@ -1856,6 +1869,8 @@ void WINAPI serviceMain(DWORD argc, LPTSTR *argv)
         apxLogWrite(APXLOG_MARK_DEBUG "Waiting 1 minute for all threads to exit.");
         reportServiceStatus(SERVICE_STOP_PENDING, NO_ERROR, ONE_MINUTE);
         apxDestroyJvm(ONE_MINUTE);
+        /* if we are not using JAVA apxDestroyJvm does nothing, check the chid processes in case they hang */
+        apxProcessTerminateChild( GetCurrentProcessId(), FALSE); /* FALSE kills! */
     }
     else {
         /* We came here without shutdown event
diff --git a/src/native/windows/include/rprocess.h b/src/native/windows/include/rprocess.h
index 934b4fc..b2ba857 100644
--- a/src/native/windows/include/rprocess.h
+++ b/src/native/windows/include/rprocess.h
@@ -45,6 +45,8 @@ DWORD       apxProcessWait(APXHANDLE hProcess, DWORD dwMilliseconds,
 BOOL        apxProcessRunning(APXHANDLE hProcess);
 DWORD       apxProcessGetPid(APXHANDLE hProcess);
 
+BOOL apxProcessTerminateChild(DWORD dwProcessId, BOOL dryrun);
+
 
 __APXEND_DECLS
 
diff --git a/src/native/windows/src/rprocess.c b/src/native/windows/src/rprocess.c
index 6857a3d..46ca36f 100644
--- a/src/native/windows/src/rprocess.c
+++ b/src/native/windows/src/rprocess.c
@@ -300,14 +300,79 @@ cleanup:
 
     return rv;
 }
+/* Check if the process is already in the process list */
+static  BOOL __apxProcessisNotinTree(DWORD *treeProcessId, DWORD curProcessId, int n) {
+    int i;
+    for (i=0; i<n; i++) {
+        if (treeProcessId[i] == curProcessId) {
+            return FALSE;
+        }
+    }
+    return TRUE;
+}
+/* Add the process to the process list */
+static  BOOL __apxProcessaddToTree(DWORD *treeProcessId, DWORD curProcessId, int n) {
+    int i;
+    for (i=0; i<n; i++) {
+        if (treeProcessId[i] == 0) {
+            treeProcessId[i] = curProcessId;
+            return TRUE;
+        }
+    }
+    return FALSE;
+}
+/* Build a process list to kill or list for debugging */
+static  BOOL __apxProcessGetTree(DWORD dwProcessId, HANDLE hProcessSnap,  DWORD *treeProcessId, int maxProcessId) {
+    for (;;) {
+        BOOL add = FALSE;
+        PROCESSENTRY32 pe32;
+        // Set the size of the structure before using it.
+        pe32.dwSize = sizeof(PROCESSENTRY32);
+        // Retrieve information about the first process,
+        // and return if unsuccessful
+        if(!Process32First(hProcessSnap, &pe32 )) {
+            apxLogWrite(APXLOG_MARK_DEBUG "Process32First failed time for %d", dwProcessId);
+            CloseHandle(hProcessSnap);          // clean the snapshot object
+            return(FALSE);
+        }
+        for (;;) {
+            if (pe32.th32ParentProcessID == dwProcessId) {
+                if (__apxProcessisNotinTree(treeProcessId,  pe32.th32ProcessID, maxProcessId)) {
+                    apxLogWrite(APXLOG_MARK_DEBUG "PROCESS NAME:  %S", pe32.szExeFile);
+
+                    apxLogWrite(APXLOG_MARK_DEBUG "Process ID        = 0x%08X (%d)", pe32.th32ProcessID,  pe32.th32ProcessID);
+                    apxLogWrite(APXLOG_MARK_DEBUG "Thread count      = %d",   pe32.cntThreads);
+                    apxLogWrite(APXLOG_MARK_DEBUG "Parent process ID = 0x%08X (%d)", pe32.th32ParentProcessID, pe32.th32ParentProcessID);
+                    apxLogWrite(APXLOG_MARK_DEBUG "Priority base     = %d", pe32.pcPriClassBase);
+                    __apxProcessGetTree( pe32.th32ProcessID, hProcessSnap,  treeProcessId, maxProcessId);
+                    __apxProcessaddToTree(treeProcessId,  pe32.th32ProcessID, maxProcessId);
+                    add = TRUE;
+                    break; /* restart the loop */
+                }
+           }
+           if (!Process32Next(hProcessSnap, &pe32)) {
+               break; /* done */
+           }
+        }
+	if (!add) {
+           break;
+        }
+    }
+    return(TRUE);
+}
 /* Terminate child processes if any
  * dwProcessId : the parent process
  * dryrun : Don't kill, just list process in debug output file.
  */
-static BOOL __apxProcessTerminateChild(DWORD dwProcessId, BOOL dryrun)
+BOOL apxProcessTerminateChild(DWORD dwProcessId, BOOL dryrun)
 {
     HANDLE hProcessSnap;
-    PROCESSENTRY32 pe32;
+    DWORD treeProcessId[32];
+    int maxProcessId = 32;
+    int i;
+    for (i=0; i<maxProcessId; i++) {
+       treeProcessId[i] = 0;
+    }
 
     apxLogWrite(APXLOG_MARK_DEBUG "TerminateChild 0x%08X (%d)", dwProcessId, dwProcessId );
     // Take a snapshot of all processes in the system.
@@ -317,46 +382,39 @@ static BOOL __apxProcessTerminateChild(DWORD dwProcessId, BOOL dryrun)
         return(FALSE);
     }
 
-    // Set the size of the structure before using it.
-    pe32.dwSize = sizeof(PROCESSENTRY32);
-
-    // Retrieve information about the first process,
-    // and return if unsuccessful
-    if( !Process32First(hProcessSnap, &pe32 )) {
-        apxLogWrite(APXLOG_MARK_DEBUG "Process32First failed");
+    // Read recursily all the child process id.
+    // display information about each process in turn if debug.
+    if (!__apxProcessGetTree(dwProcessId, hProcessSnap,  treeProcessId, maxProcessId)) {
+        apxLogWrite(APXLOG_MARK_DEBUG "__apxProcessGetTree failed");
         CloseHandle(hProcessSnap);          // clean the snapshot object
         return(FALSE);
     }
 
-    // Now walk the snapshot of processes, and
+    // kill all the processes we have discover.
+     
     // display information about each process in turn
-    do {
-        if (pe32.th32ParentProcessID == dwProcessId) {
-            apxLogWrite(APXLOG_MARK_DEBUG "PROCESS NAME:  %S", pe32.szExeFile);
-
-            apxLogWrite(APXLOG_MARK_DEBUG "Process ID        = 0x%08X", pe32.th32ProcessID);
-            apxLogWrite(APXLOG_MARK_DEBUG "Thread count      = %d",   pe32.cntThreads);
-            apxLogWrite(APXLOG_MARK_DEBUG "Parent process ID = 0x%08X", pe32.th32ParentProcessID);
-            apxLogWrite(APXLOG_MARK_DEBUG "Priority base     = %d", pe32.pcPriClassBase);
-            if (!dryrun) {
-                HANDLE hProcess;
-                hProcess = OpenProcess(PROCESS_ALL_ACCESS, FALSE, pe32.th32ProcessID);
-                if(hProcess != NULL) {
-                   TerminateProcess(hProcess, CHILD_TERMINATE_CODE);
-                   apxLogWrite(APXLOG_MARK_DEBUG "Process ID: 0x%08X (%d) Terminated!", pe32.th32ProcessID, pe32.th32ProcessID);
-                   CloseHandle(hProcess);
-                } else {
-                   apxLogWrite(APXLOG_MARK_DEBUG "Process ID: 0x%08X (%d) Termination failed!", pe32.th32ProcessID, pe32.th32ProcessID);
-                }
-            }
+    if (!dryrun) {
+        HANDLE hProcess;
+        for (i=0; i<maxProcessId; i++) {
+           if (treeProcessId[i] == 0) {
+              break; /* Done */
+           }
+           hProcess = OpenProcess(PROCESS_ALL_ACCESS, FALSE, treeProcessId[i]);
+           if(hProcess != NULL) {
+              TerminateProcess(hProcess, CHILD_TERMINATE_CODE);
+              apxLogWrite(APXLOG_MARK_DEBUG "Process ID: 0x%08X (%d) Terminated!", treeProcessId[i], treeProcessId[i]);
+              CloseHandle(hProcess);
+           } else {
+              apxLogWrite(APXLOG_MARK_DEBUG "Process ID: 0x%08X (%d) Termination failed!",  treeProcessId[i], treeProcessId[i]);
+           }
        }
+    }
 
-    } while(Process32Next(hProcessSnap, &pe32));
-  
     CloseHandle(hProcessSnap);
     return(TRUE);
 }
 
+
 /* Close the process.
  * Create the remote thread and call the ExitProcess
  * Terminate the process, if all of the above fails.
@@ -374,7 +432,7 @@ static BOOL __apxProcessClose(APXHANDLE hProcess)
     CHECK_IF_ACTIVE(lpProc);
 
     /* dry run to get debug information */
-    __apxProcessTerminateChild(lpProc->stProcInfo.dwProcessId, TRUE);
+    apxProcessTerminateChild(lpProc->stProcInfo.dwProcessId, TRUE);
     /* Try to close the child's stdin first */
     SAFE_CLOSE_HANDLE(lpProc->hChildInpWr);
     /* Wait 1 sec for child process to
@@ -413,7 +471,7 @@ static BOOL __apxProcessClose(APXHANDLE hProcess)
                      /* We are here when the service starts something like wildfly via standalone.sh and wildfly doesn't terminate cleanly, 
                       * dry run is FALSE: we kill all the process of the process tree
                       */
-                     __apxProcessTerminateChild(lpProc->stProcInfo.dwProcessId, FALSE);
+                     apxProcessTerminateChild(lpProc->stProcInfo.dwProcessId, FALSE);
                 }
 
 
@@ -804,6 +862,8 @@ apxProcessWait(APXHANDLE hProcess, DWORD dwMilliseconds, BOOL bKill)
         if (rv == WAIT_TIMEOUT && bKill) {
             apxLogWrite(APXLOG_MARK_DEBUG "apxProcessWait. killing???");
             __apxProcessCallback(hProcess, WM_CLOSE, 0, 0);
+            apxLogWrite(APXLOG_MARK_DEBUG "apxProcessWait. killing??? after WM_CLOSE");
+            apxProcessTerminateChild(GetCurrentProcessId(), TRUE); 
         }
         return rv;
     }