Fix wxKill(wxSIGNONE) always returning true for child processes in wxMSW.

The fact that a handle to a process can be opened doesn't mean that the
process is still running. In fact, for a child process that we store a handle
for ourselves we will always be able to open (another copy of the) handle even
if it already terminated.

Check for the process termination using WaitForSingleObject() instead in both
normal and wxSIGNONE cases.

Also simplify the code by not using GetExitCodeProcess() at all as we don't
need the process exit code.

Closes #2834.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@65493 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

diff --git a/src/msw/utils.cpp b/src/msw/utils.cpp
index 47009f0bd093211fafc2bd3e4f5cb9cdadd6eead..a456a3417a654ae979bf7a88c6b87d55b68d591a 100644 (file)
--- a/src/msw/utils.cpp
+++ b/src/msw/utils.cpp
@@ -730,6 +730,12 @@ int wxKill(long pid, wxSignal sig, wxKillError *krc, int flags)
 
     wxON_BLOCK_EXIT1(::CloseHandle, hProcess);
 
+    // Default timeout for waiting for the process termination after killing
+    // it. It should be long enough to allow the process to terminate even on a
+    // busy system but short enough to avoid blocking the main thread for too
+    // long.
+    DWORD waitTimeout = 500; // ms
+
     bool ok = true;
     switch ( sig )
     {
@@ -751,10 +757,14 @@ int wxKill(long pid, wxSignal sig, wxKillError *krc, int flags)
             break;
 
         case wxSIGNONE:
-            // do nothing, we just want to test for process existence
-            if ( krc )
-                *krc = wxKILL_OK;
-            return 0;
+            // Opening the process handle may succeed for a process even if it
+            // doesn't run any more (typically because open handles to it still
+            // exist elsewhere, possibly in this process itself if we're
+            // killing a child process) so we still need check if it hasn't
+            // terminated yet but, unlike when killing it, we don't need to
+            // wait for any time at all.
+            waitTimeout = 0;
+            break;
 
         default:
             // any other signal means "terminate"
@@ -799,18 +809,22 @@ int wxKill(long pid, wxSignal sig, wxKillError *krc, int flags)
     }
 
     // the return code
-    DWORD rc wxDUMMY_INITIALIZE(0);
     if ( ok )
     {
         // as we wait for a short time, we can use just WaitForSingleObject()
         // and not MsgWaitForMultipleObjects()
-        switch ( ::WaitForSingleObject(hProcess, 500 /* msec */) )
+        switch ( ::WaitForSingleObject(hProcess, waitTimeout) )
         {
             case WAIT_OBJECT_0:
-                // process terminated
-                if ( !::GetExitCodeProcess(hProcess, &rc) )
+                // Process terminated: normally this indicates that we
+                // successfully killed it but when testing for the process
+                // existence, this means failure.
+                if ( sig == wxSIGNONE )
                 {
-                    wxLogLastError(wxT("GetExitCodeProcess"));
+                    if ( krc )
+                        *krc = wxKILL_NO_PROCESS;
+
+                    ok = false;
                 }
                 break;
 
@@ -823,10 +837,15 @@ int wxKill(long pid, wxSignal sig, wxKillError *krc, int flags)
                 // fall through
 
             case WAIT_TIMEOUT:
-                if ( krc )
-                    *krc = wxKILL_ERROR;
+                // Process didn't terminate: normally this is a failure but not
+                // when we're just testing for its existence.
+                if ( sig != wxSIGNONE )
+                {
+                    if ( krc )
+                        *krc = wxKILL_ERROR;
 
-                rc = STILL_ACTIVE;
+                    ok = false;
+                }
                 break;
         }
     }
@@ -834,7 +853,7 @@ int wxKill(long pid, wxSignal sig, wxKillError *krc, int flags)
 
     // the return code is the same as from Unix kill(): 0 if killed
     // successfully or -1 on error
-    if ( !ok || rc == STILL_ACTIVE )
+    if ( !ok )
         return -1;
 
     if ( krc )