Skip to content

Commit

Permalink
Add ChildProcessTerminationInfo
Browse files Browse the repository at this point in the history
Combine termination status and exit code into a single struct. And then
add two Android-only fields: oom protection binding, and intentional
kill.

Update the code in ChildProcessLauncher and ChildProcessLauncherHelper
to populate Info instead, including renaming methods.

Other than populating the new fields on Android, this is a no-op change
on all platforms.

Bug: 693484
Change-Id: I1ce01cbf1e81a125b34604657d6abcb6a02e0556
Reviewed-on: https://chromium-review.googlesource.com/1013225
Reviewed-by: Mark Seaborn <mseaborn@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551894}
  • Loading branch information
Bo Liu authored and Commit Bot committed Apr 19, 2018
1 parent 44dabb5 commit 0d2a232
Show file tree
Hide file tree
Showing 17 changed files with 179 additions and 123 deletions.
8 changes: 4 additions & 4 deletions components/nacl/browser/nacl_process_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -252,12 +252,12 @@ NaClProcessHost::NaClProcessHost(
NaClProcessHost::~NaClProcessHost() {
// Report exit status only if the process was successfully started.
if (process_->GetData().handle != base::kNullProcessHandle) {
int exit_code = 0;
process_->GetTerminationStatus(false /* known_dead */, &exit_code);
content::ChildProcessTerminationInfo info =
process_->GetTerminationInfo(false /* known_dead */);
std::string message =
base::StringPrintf("NaCl process exited with status %i (0x%x)",
exit_code, exit_code);
if (exit_code == 0) {
info.exit_code, info.exit_code);
if (info.exit_code == 0) {
VLOG(1) << message;
} else {
LOG(ERROR) << message;
Expand Down
32 changes: 17 additions & 15 deletions content/browser/browser_child_process_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -358,13 +358,16 @@ void BrowserChildProcessHostImpl::HistogramBadMessageTerminated(
PROCESS_TYPE_MAX);
}

base::TerminationStatus BrowserChildProcessHostImpl::GetTerminationStatus(
bool known_dead, int* exit_code) {
ChildProcessTerminationInfo BrowserChildProcessHostImpl::GetTerminationInfo(
bool known_dead) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
if (!child_process_) // If the delegate doesn't use Launch() helper.
return base::GetTerminationStatus(data_.handle, exit_code);
return child_process_->GetChildTerminationStatus(known_dead,
exit_code);
if (!child_process_) {
// If the delegate doesn't use Launch() helper.
ChildProcessTerminationInfo info;
info.status = base::GetTerminationStatus(data_.handle, &info.exit_code);
return info;
}
return child_process_->GetChildTerminationInfo(known_dead);
}

bool BrowserChildProcessHostImpl::OnMessageReceived(
Expand Down Expand Up @@ -439,16 +442,15 @@ void BrowserChildProcessHostImpl::OnChildDisconnected() {
early_exit_watcher_.StopWatching();
#endif
if (child_process_.get() || data_.handle) {
int exit_code;
base::TerminationStatus status = GetTerminationStatus(
true /* known_dead */, &exit_code);
switch (status) {
ChildProcessTerminationInfo info =
GetTerminationInfo(true /* known_dead */);
switch (info.status) {
case base::TERMINATION_STATUS_PROCESS_CRASHED:
case base::TERMINATION_STATUS_ABNORMAL_TERMINATION: {
delegate_->OnProcessCrashed(exit_code);
delegate_->OnProcessCrashed(info.exit_code);
BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE,
base::BindOnce(&NotifyProcessCrashed, data_, exit_code));
base::BindOnce(&NotifyProcessCrashed, data_, info.exit_code));
UMA_HISTOGRAM_ENUMERATION("ChildProcess.Crashed2",
static_cast<ProcessType>(data_.process_type),
PROCESS_TYPE_MAX);
Expand All @@ -461,10 +463,10 @@ void BrowserChildProcessHostImpl::OnChildDisconnected() {
case base::TERMINATION_STATUS_PROCESS_WAS_KILLED_BY_OOM:
#endif
case base::TERMINATION_STATUS_PROCESS_WAS_KILLED: {
delegate_->OnProcessCrashed(exit_code);
delegate_->OnProcessCrashed(info.exit_code);
BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE,
base::BindOnce(&NotifyProcessKilled, data_, exit_code));
base::BindOnce(&NotifyProcessKilled, data_, info.exit_code));
// Report that this child process was killed.
UMA_HISTOGRAM_ENUMERATION("ChildProcess.Killed2",
static_cast<ProcessType>(data_.process_type),
Expand All @@ -484,7 +486,7 @@ void BrowserChildProcessHostImpl::OnChildDisconnected() {
static_cast<ProcessType>(data_.process_type),
PROCESS_TYPE_MAX);
#if defined(OS_CHROMEOS)
if (status == base::TERMINATION_STATUS_PROCESS_WAS_KILLED_BY_OOM) {
if (info.status == base::TERMINATION_STATUS_PROCESS_WAS_KILLED_BY_OOM) {
UMA_HISTOGRAM_ENUMERATION("ChildProcess.Killed2.OOM",
static_cast<ProcessType>(data_.process_type),
PROCESS_TYPE_MAX);
Expand Down
3 changes: 1 addition & 2 deletions content/browser/browser_child_process_host_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,7 @@ class CONTENT_EXPORT BrowserChildProcessHostImpl
bool terminate_on_shutdown) override;
const ChildProcessData& GetData() const override;
ChildProcessHost* GetHost() const override;
base::TerminationStatus GetTerminationStatus(bool known_dead,
int* exit_code) override;
ChildProcessTerminationInfo GetTerminationInfo(bool known_dead) override;
std::unique_ptr<base::SharedPersistentMemoryAllocator> TakeMetricsAllocator()
override;
void SetName(const base::string16& name) override;
Expand Down
32 changes: 12 additions & 20 deletions content/browser/child_process_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ ChildProcessLauncher::ChildProcessLauncher(
const mojo::edk::ProcessErrorCallback& process_error_callback,
bool terminate_on_shutdown)
: client_(client),
termination_status_(base::TERMINATION_STATUS_NORMAL_TERMINATION),
exit_code_(RESULT_CODE_NORMAL_EXIT),
starting_(true),
#if defined(ADDRESS_SANITIZER) || defined(LEAK_SANITIZER) || \
defined(MEMORY_SANITIZER) || defined(THREAD_SANITIZER) || \
Expand Down Expand Up @@ -80,7 +78,7 @@ void ChildProcessLauncher::Notify(
if (process_.process.IsValid()) {
client_->OnProcessLaunched();
} else {
termination_status_ = base::TERMINATION_STATUS_LAUNCH_FAILED;
termination_info_.status = base::TERMINATION_STATUS_LAUNCH_FAILED;

// NOTE: May delete |this|.
client_->OnProcessLaunchFailed(error_code);
Expand All @@ -99,40 +97,34 @@ const base::Process& ChildProcessLauncher::GetProcess() const {
return process_.process;
}

base::TerminationStatus ChildProcessLauncher::GetChildTerminationStatus(
bool known_dead,
int* exit_code) {
ChildProcessTerminationInfo ChildProcessLauncher::GetChildTerminationInfo(
bool known_dead) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

if (!process_.process.IsValid()) {
// Make sure to avoid using the default termination status if the process
// hasn't even started yet.
if (IsStarting())
termination_status_ = base::TERMINATION_STATUS_STILL_RUNNING;
termination_info_.status = base::TERMINATION_STATUS_STILL_RUNNING;

// Process doesn't exist, so return the cached termination status.
if (exit_code)
*exit_code = exit_code_;
return termination_status_;
// Process doesn't exist, so return the cached termination info.
return termination_info_;
}

termination_status_ =
helper_->GetTerminationStatus(process_, known_dead, &exit_code_);
if (exit_code)
*exit_code = exit_code_;
termination_info_ = helper_->GetTerminationInfo(process_, known_dead);

// POSIX: If the process crashed, then the kernel closed the socket for it and
// so the child has already died by the time we get here. Since
// GetTerminationStatus called waitpid with WNOHANG, it'll reap the process.
// However, if GetTerminationStatus didn't reap the child (because it was
// GetTerminationInfo called waitpid with WNOHANG, it'll reap the process.
// However, if GetTerminationInfo didn't reap the child (because it was
// still running), we'll need to Terminate via ProcessWatcher. So we can't
// close the handle here.
if (termination_status_ != base::TERMINATION_STATUS_STILL_RUNNING) {
process_.process.Exited(exit_code_);
if (termination_info_.status != base::TERMINATION_STATUS_STILL_RUNNING) {
process_.process.Exited(termination_info_.exit_code);
process_.process.Close();
}

return termination_status_;
return termination_info_;
}

bool ChildProcessLauncher::Terminate(int exit_code) {
Expand Down
10 changes: 3 additions & 7 deletions content/browser/child_process_launcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "content/browser/child_process_launcher_helper.h"
#include "content/common/content_export.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/child_process_termination_info.h"
#include "content/public/common/result_codes.h"
#include "mojo/edk/embedder/outgoing_broker_client_invitation.h"
#include "mojo/edk/embedder/scoped_platform_handle.h"
Expand Down Expand Up @@ -118,11 +119,7 @@ class CONTENT_EXPORT ChildProcessLauncher {
// process could be seen as running. With |known_dead| set to true, the
// process will be killed if it was still running. See ZygoteHostImpl for
// more discussion of Linux implementation details.
// |exit_code| is the exit code of the process if it exited (e.g. status from
// waitpid if on posix, from GetExitCodeProcess on Windows). |exit_code| may
// be NULL.
base::TerminationStatus GetChildTerminationStatus(bool known_dead,
int* exit_code);
ChildProcessTerminationInfo GetChildTerminationInfo(bool known_dead);

// Changes whether the process runs in the background or not. Only call
// this after the process has started.
Expand Down Expand Up @@ -172,8 +169,7 @@ class CONTENT_EXPORT ChildProcessLauncher {
// ChildProcessLauncherHelper once the process was started.
internal::ChildProcessLauncherHelper::Process process_;

base::TerminationStatus termination_status_;
int exit_code_;
ChildProcessTerminationInfo termination_info_;
bool starting_;

// Controls whether the child process should be terminated on browser
Expand Down
9 changes: 4 additions & 5 deletions content/browser/child_process_launcher_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ namespace content {
class ChildProcessLauncher;
class SandboxedProcessLauncherDelegate;
struct ChildProcessLauncherPriority;
struct ChildProcessTerminationInfo;

#if defined(OS_POSIX)
class PosixFileDescriptorInfo;
Expand Down Expand Up @@ -146,12 +147,10 @@ class ChildProcessLauncherHelper :

int client_thread_id() const { return client_thread_id_; }

// Returns the termination status and sets |exit_code| if non null.
// See ChildProcessLauncher::GetChildTerminationStatus for more info.
base::TerminationStatus GetTerminationStatus(
// See ChildProcessLauncher::GetChildTerminationInfo for more info.
ChildProcessTerminationInfo GetTerminationInfo(
const ChildProcessLauncherHelper::Process& process,
bool known_dead,
int* exit_code);
bool known_dead);

// Terminates |process|.
// Returns true if the process was stopped, false if the process had not been
Expand Down
32 changes: 22 additions & 10 deletions content/browser/child_process_launcher_helper_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -149,18 +149,30 @@ void ChildProcessLauncherHelper::AfterLaunchOnLauncherThread(
const base::LaunchOptions& options) {
}

base::TerminationStatus ChildProcessLauncherHelper::GetTerminationStatus(
ChildProcessTerminationInfo ChildProcessLauncherHelper::GetTerminationInfo(
const ChildProcessLauncherHelper::Process& process,
bool known_dead,
int* exit_code) {
if (java_peer_avaiable_on_client_thread_ &&
Java_ChildProcessLauncherHelper_isOomProtected(AttachCurrentThread(),
java_peer_)) {
return base::TERMINATION_STATUS_OOM_PROTECTED;
bool known_dead) {
ChildProcessTerminationInfo info;
info.has_oom_protection_bindings =
java_peer_avaiable_on_client_thread_ &&
Java_ChildProcessLauncherHelper_hasOomProtectionBindings(
AttachCurrentThread(), java_peer_);
info.was_killed_intentionally_by_browser =
java_peer_avaiable_on_client_thread_ &&
Java_ChildProcessLauncherHelper_isKilledByUs(AttachCurrentThread(),
java_peer_);
bool app_foreground =
java_peer_avaiable_on_client_thread_ &&
Java_ChildProcessLauncherHelper_isApplicationInForeground(
AttachCurrentThread(), java_peer_);
if (app_foreground && info.has_oom_protection_bindings) {
info.status = base::TERMINATION_STATUS_OOM_PROTECTED;
} else {
// Note waitpid does not work on Android since these are not actually child
// processes. So there is no need for base::GetTerminationInfo.
info.status = base::TERMINATION_STATUS_NORMAL_TERMINATION;
}
// Note waitpid does not work on Android since these are not actually child
// processes. So there is no need for base::GetTerminationStatus.
return base::TERMINATION_STATUS_NORMAL_TERMINATION;
return info;
}

// static
Expand Down
10 changes: 6 additions & 4 deletions content/browser/child_process_launcher_helper_fuchsia.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ void ChildProcessLauncherHelper::SetProcessPriorityOnLauncherThread(
NOTIMPLEMENTED();
}

base::TerminationStatus ChildProcessLauncherHelper::GetTerminationStatus(
ChildProcessTerminationInfo ChildProcessLauncherHelper::GetTerminationInfo(
const ChildProcessLauncherHelper::Process& process,
bool known_dead,
int* exit_code) {
return base::GetTerminationStatus(process.process.Handle(), exit_code);
bool known_dead) {
ChildProcessTerminationInfo info;
info.status =
base::GetTerminationStatus(process.process.Handle(), &info.exit_code);
return info;
}

// static
Expand Down
22 changes: 12 additions & 10 deletions content/browser/child_process_launcher_helper_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -115,19 +115,21 @@ void ChildProcessLauncherHelper::AfterLaunchOnLauncherThread(
const base::LaunchOptions& options) {
}

base::TerminationStatus ChildProcessLauncherHelper::GetTerminationStatus(
ChildProcessTerminationInfo ChildProcessLauncherHelper::GetTerminationInfo(
const ChildProcessLauncherHelper::Process& process,
bool known_dead,
int* exit_code) {
bool known_dead) {
ChildProcessTerminationInfo info;
if (process.zygote) {
return process.zygote->GetTerminationStatus(
process.process.Handle(), known_dead, exit_code);
}
if (known_dead) {
return base::GetKnownDeadTerminationStatus(
process.process.Handle(), exit_code);
info.status = process.zygote->GetTerminationStatus(
process.process.Handle(), known_dead, &info.exit_code);
} else if (known_dead) {
info.status = base::GetKnownDeadTerminationStatus(process.process.Handle(),
&info.exit_code);
} else {
info.status =
base::GetTerminationStatus(process.process.Handle(), &info.exit_code);
}
return base::GetTerminationStatus(process.process.Handle(), exit_code);
return info;
}

// static
Expand Down
14 changes: 8 additions & 6 deletions content/browser/child_process_launcher_helper_mac.cc
Original file line number Diff line number Diff line change
Expand Up @@ -223,13 +223,15 @@ void ChildProcessLauncherHelper::AfterLaunchOnLauncherThread(
broker->GetLock().Release();
}

base::TerminationStatus ChildProcessLauncherHelper::GetTerminationStatus(
ChildProcessTerminationInfo ChildProcessLauncherHelper::GetTerminationInfo(
const ChildProcessLauncherHelper::Process& process,
bool known_dead,
int* exit_code) {
return known_dead
? base::GetKnownDeadTerminationStatus(process.process.Handle(), exit_code)
: base::GetTerminationStatus(process.process.Handle(), exit_code);
bool known_dead) {
ChildProcessTerminationInfo info;
info.status = known_dead ? base::GetKnownDeadTerminationStatus(
process.process.Handle(), &info.exit_code)
: base::GetTerminationStatus(
process.process.Handle(), &info.exit_code);
return info;
}

// static
Expand Down
10 changes: 6 additions & 4 deletions content/browser/child_process_launcher_helper_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,13 @@ void ChildProcessLauncherHelper::AfterLaunchOnLauncherThread(
DCHECK(CurrentlyOnProcessLauncherTaskRunner());
}

base::TerminationStatus ChildProcessLauncherHelper::GetTerminationStatus(
ChildProcessTerminationInfo ChildProcessLauncherHelper::GetTerminationInfo(
const ChildProcessLauncherHelper::Process& process,
bool known_dead,
int* exit_code) {
return base::GetTerminationStatus(process.process.Handle(), exit_code);
bool known_dead) {
ChildProcessTerminationInfo info;
info.status =
base::GetTerminationStatus(process.process.Handle(), &info.exit_code);
return info;
}

// static
Expand Down
Loading

0 comments on commit 0d2a232

Please sign in to comment.