check for successful exit code in WaitForSingleProcess on Windows to

base/process_util_win.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/process_util.h"
 
 #include <fcntl.h>
 #include <io.h>
 #include <windows.h>
 #include <userenv.h>
@@ skipping 589 matching lines @@
                                  entry->th32ProcessID);
     DWORD wait_result = WaitForSingleObject(process, remaining_wait);
     CloseHandle(process);
     result = result && (wait_result == WAIT_OBJECT_0);
   }
 
   return result;
 }
 
 bool WaitForSingleProcess(ProcessHandle handle, int64 wait_milliseconds) {
-  bool retval = WaitForSingleObject(handle, wait_milliseconds) == WAIT_OBJECT_0;
-  return retval;
+  int exit_code;
+  if (!WaitForExitCodeWithTimeout(handle, &exit_code, wait_milliseconds))
+    return false;
+  return exit_code == 0;

[Reid Kleckner, 2011/12/15 22:14:19]

This has slightly different semantics from the POSIX version of
WaitForSingleProcess.  That one uses "return WIFEXITED(exit_code)", which will
return true even if the process exits non-zero.  This should return false only
if the child crashed.

[bruening, 2011/12/16 04:08:06]

The problem is that on Windows exit codes set by the app can be any 32-bit value
so you're never really sure whether the app really crashed or it just used an
unusual exit code.  I suppose this could check for some standard values like
0xc0000005 or 0xc000001d but there are probably 20 or so different codes that
can constitute a crash and they aren't contiguous.  OTOH, no normal exit code is
likely to be in the STATUS_/EXCEPTION_ range.  The docs for this function talk
about a "clean exit": so checking for actual success seems reasonable.  It
depends on how this is meant to be used.  I would lean toward failing on
non-zero to catch more bugs unless we see existing usage that this breaks, which
so far has not been the case.

[Reid Kleckner, 2011/12/20 23:43:24]

I was going to say that the same thing can happen on POSIX, but they do the
return code << 8 dance to make space for the signal bits.  Checking for != 0 is
most conservative, so this seems reasonable.

 }
 
 bool CleanupProcesses(const std::wstring& executable_name,
                       int64 wait_milliseconds,
                       int exit_code,
                       const ProcessFilter* filter) {
   bool exited_cleanly = WaitForProcessesToExit(executable_name,
                                                wait_milliseconds,
                                                filter);
   if (!exited_cleanly)
@@ skipping 368 matching lines @@
 
   PERFORMANCE_INFORMATION info;
   if (!InternalGetPerformanceInfo(&info, sizeof(info))) {
     DLOG(ERROR) << "Failed to fetch internal performance info.";
     return 0;
   }
   return (info.CommitTotal * system_info.dwPageSize) / 1024;
 }
 
 }  // namespace base