Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(145)

Unified Diff: sandbox/win/src/target_process.cc

Issue 2517703003: Terminate child process if sandbox::TargetProcess is destroyed. (Closed)
Patch Set: change result code Created 4 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: sandbox/win/src/target_process.cc
diff --git a/sandbox/win/src/target_process.cc b/sandbox/win/src/target_process.cc
index 72e2780c8c3fff520c93704b4ca4b3c66e1a4283..d163cfb5504840d4308607420ce78bc67b08c476 100644
--- a/sandbox/win/src/target_process.cc
+++ b/sandbox/win/src/target_process.cc
@@ -63,30 +63,17 @@ TargetProcess::TargetProcess(base::win::ScopedHandle initial_token,
base_address_(NULL) {}
TargetProcess::~TargetProcess() {
- DWORD exit_code = 0;
// Give a chance to the process to die. In most cases the JOB_KILL_ON_CLOSE
// will take effect only when the context changes. As far as the testing went,
// this wait was enough to switch context and kill the processes in the job.
// If this process is already dead, the function will return without waiting.
- // TODO(nsylvain): If the process is still alive at the end, we should kill
- // it. http://b/893891
// For now, this wait is there only to do a best effort to prevent some leaks
// from showing up in purify.
if (sandbox_process_info_.IsValid()) {
::WaitForSingleObject(sandbox_process_info_.process_handle(), 50);
- // At this point, the target process should have been killed. Check.
- if (!::GetExitCodeProcess(sandbox_process_info_.process_handle(),
- &exit_code) || (STILL_ACTIVE == exit_code)) {
- // Something went wrong. We don't know if the target is in a state where
- // it can manage to do another IPC call. If it can, and we've destroyed
- // the |ipc_server_|, it will crash the broker. So we intentionally leak
- // that.
- if (shared_section_.IsValid())
- shared_section_.Take();
- ignore_result(ipc_server_.release());
- sandbox_process_info_.TakeProcessHandle();
- return;
- }
+ // Terminate the process if it's still alive, as its IPC server is going
+ // away. 1 is RESULT_CODE_KILLED.
+ ::TerminateProcess(sandbox_process_info_.process_handle(), 1);
}
// ipc_server_ references our process handle, so make sure the former is shut
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698