This adds some plumbing for propagating the reason for a renderer's death

base/process_util.h

 // Copyright (c) 2010 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.
 
 // This file/namespace contains utility functions for enumerating, ending and
 // computing statistics of processes.
 
 #ifndef BASE_PROCESS_UTIL_H_
 #define BASE_PROCESS_UTIL_H_
 #pragma once
@@ skipping 100 matching lines @@
 const uint32 kProcessAccessDuplicateHandle        = 0;
 const uint32 kProcessAccessCreateProcess          = 0;
 const uint32 kProcessAccessSetQuota               = 0;
 const uint32 kProcessAccessSetInformation         = 0;
 const uint32 kProcessAccessQueryInformation       = 0;
 const uint32 kProcessAccessSuspendResume          = 0;
 const uint32 kProcessAccessQueryLimitedInfomation = 0;
 const uint32 kProcessAccessWaitForTermination     = 0;
 #endif  // defined(OS_POSIX)
 
-// A minimalistic but hopefully cross-platform set of exit codes.
-// Do not change the enumeration values or you will break third-party
-// installers.
-enum {
-  PROCESS_END_NORMAL_TERMINATION = 0,
-  PROCESS_END_KILLED_BY_USER     = 1,
-  PROCESS_END_PROCESS_WAS_HUNG   = 2
+// A minimalistic but hopefully cross-platform set of exit codes.  Do
+// not change the enumeration values or you will break third-party
+// installers.  You may use these as arguments for KillProcess*()
+// (among other uses).
+enum ExitCode {

[brettw, 2010/12/03 01:01:54]

I'm kind of sad to see this enumeration here (I realize you didn't add it). This
kind of thing doesn't belong in base isn't the right place to force meanings of
exit codes.

I did some poking and there are no dependencies in base outside of
base::DidProcessCrash, and this function is not used outside of Chrome and then
always in the context of ChildProcessLauncher. Can we just move DidProcessCrash
to a static function in ChildProcessLauncher and then just delete this enum
(using the big ResultCodes enum instead).

[Greg Spencer (Chromium), 2010/12/03 18:29:25]

Yeah, I'm not a big fan of this enum either, but the comment was scary enough to
me that I didn't want to just eliminate it.  I'd be happy to get rid of it
though (I actually did that initially, and put it back to differentiate the
interpreted status from the exit codes).

So, just so I'm clear on what you're suggesting here:  You'd like me to move
GetTerminationStatus (the renamed DidProcessCrash) into ChildProcessLauncher,
eliminate the ExitCode enum, and replace usage of the EXIT_CODE_* values in the
code with the equivalent value in the TerminationStatus enum (which I presume
you'd want moved to ChildProcessLauncher as well) so that when KillProcess is
called (especially on Windows), it uses the same 0, 1, 2 values to keep the
third party installers from breaking.

It looks to me like zygote_main_linux.cc uses GetTerminationStatus outside of
the ChildProcessLauncher, so there will have to be a static.  So does
nacl_process_host.cc (when on wow64).

I actually think that this function belongs here, though.  (With the exception
of the exit code crap) it's not specific to Chrome, even if Chrome is the only
client right now.  How strongly do you feel about moving it? :)  It seems to fit
well with the other process utilities here.

[brettw, 2010/12/09 16:48:49]

I don't really like the exit codes here, personally. Lemme talk to some
different people today to see what the opinion is about this.

[brettw, 2010/12/09 17:22:13]

I'm more sure that I don't want this in base.

The main point is that no non-Chrome processes use these codes. This means that
having this in base is actively misleading. For example, none of the test
executables use this, they return 0 on success and 1 on failure. This
corresponds to WAS_KILLED. Another example is the stress_cache executable in
net, which returns 0 on success and -1 on failure.

The stuff in base should be always useful. But in any of these cases, if I call
GetTerminationStatus, the result will be incorrect, despite the function looking
like it will be checking something for me.

Previously, I think the use of these was a bit borderline. They were there, and
you could call WaitForExitCode to get the exit code. I would still have
preferred it to be in common for the reasons above. But with your change and the
addition of GetTerminationStatus, I think the code here is more actively
confusing about how exit codes are handled.

The situation we have now is that each process decides what its exit codes are.
For Chrome sub-processes, we have a scheme that we use, and the code and enums
that go with that scheme should be in Chrome.

[Greg Spencer (Chromium), 2010/12/09 17:58:42]

OK, as I said, I'm happy taking the exit code out and moving it to
ChildProcessLauncher (or wherever).

The thing I'd like to leave in base is GetTerminationStatus, since it's telling
us something generic about processes and signals, not relying on chrome-specific
exit codes.

+  EXIT_CODE_NORMAL_TERMINATION = 0,
+  EXIT_CODE_PROCESS_WAS_KILLED = 1,
+  EXIT_CODE_PROCESS_WAS_HUNG = 2,
+};
+
+// Return status values from GetTerminationStatus.  Don't use these as
+// arguments to KillProcess*(), use ExitCode values above instead.
+enum TerminationStatus {
+  TERMINATION_STATUS_NORMAL_TERMINATION = 0,  // zero exit status
+  TERMINATION_STATUS_PROCESS_WAS_KILLED = 1,  // e.g. SIGKILL or task manager
+  TERMINATION_STATUS_PROCESS_WAS_HUNG = 2,
+  TERMINATION_STATUS_PROCESS_CRASHED = 3,  // e.g. Segmentation fault
+  TERMINATION_STATUS_ABNORMAL_TERMINATION = 4,  // non-zero exit status
+  TERMINATION_STATUS_STILL_RUNNING = 5  // child hasn't exited yet
 };
 
 // Returns the id of the current process.
 ProcessId GetCurrentProcId();
 
 // Returns the ProcessHandle of the current process.
 ProcessHandle GetCurrentProcessHandle();
 
 // Converts a PID to a process handle. This handle must be closed by
 // CloseProcessHandle when you are done with it. Returns true on success.
@@ skipping 35 matching lines @@
 
 static const char kAdjustOOMScoreSwitch[] = "--adjust-oom-score";
 
 // This adjusts /proc/process/oom_adj so the Linux OOM killer will prefer
 // certain process types over others. The range for the adjustment is
 // [-17,15], with [0,15] being user accessible.
 bool AdjustOOMScore(ProcessId process, int score);
 #endif
 
 #if defined(OS_POSIX)
-// Close all file descriptors, expect those which are a destination in the
+// Close all file descriptors, except those which are a destination in the
 // given multimap. Only call this function in a child process where you know
 // that there aren't any other threads.
 void CloseSuperfluousFds(const InjectiveMultimap& saved_map);
 #endif
 
 #if defined(OS_WIN)
 
 enum IntegrityLevel {
   INTEGRITY_UNKNOWN,
   LOW_INTEGRITY,
@@ skipping 146 matching lines @@
 #if defined(OS_POSIX)
 // Attempts to kill the process group identified by |process_group_id|. Returns
 // true on success.
 bool KillProcessGroup(ProcessHandle process_group_id);
 #endif
 
 #if defined(OS_WIN)
 bool KillProcessById(ProcessId process_id, int exit_code, bool wait);
 #endif
 
-// Get the termination status (exit code) of the process and return true if the
-// status indicates the process crashed. |child_exited| is set to true iff the
-// child process has terminated. (|child_exited| may be NULL.)
-bool DidProcessCrash(bool* child_exited, ProcessHandle handle);
+// Get the termination status of the process by interpreting the
+// circumstances of the child process' death. |exit_code| is set to
+// the status returned by waitpid() on POSIX, and from
+// GetExitCodeProcess() on Windows.  |exit_code| may be NULL if the
+// caller is not interested in it.
+TerminationStatus GetTerminationStatus(ProcessHandle handle, int* exit_code);
 
 // Waits for process to exit. In POSIX systems, if the process hasn't been
 // signaled then puts the exit code in |exit_code|; otherwise it's considered
 // a failure. On Windows |exit_code| is always filled. Returns true on success,
 // and closes |handle| in any case.
 bool WaitForExitCode(ProcessHandle handle, int* exit_code);
 
 // Waits for process to exit. If it did exit within |timeout_milliseconds|,
 // then puts the exit code in |exit_code|, closes |handle|, and returns true.
 // In POSIX systems, if the process has been signaled then |exit_code| is set
@@ skipping 298 matching lines @@
 // instance running inside the parent.  The parent's Breakpad instance should
 // not handle the child's exceptions.  Calling RestoreDefaultExceptionHandler
 // in the child after forking will restore the standard exception handler.
 // See http://crbug.com/20371/ for more details.
 void RestoreDefaultExceptionHandler();
 #endif  // defined(OS_MACOSX)
 
 }  // namespace base
 
 #endif  // BASE_PROCESS_UTIL_H_