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

Unified Diff: runtime/bin/process_win.cc

Issue 10825473: Fix dart:io Process class on the Windows plaform, which had some race conditions. (Closed) Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge/dart
Patch Set: Created 8 years, 4 months 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: runtime/bin/process_win.cc
diff --git a/runtime/bin/process_win.cc b/runtime/bin/process_win.cc
index e9cab3055c75061c9287cda9293179ff7bd73dc7..331807bb260ee6bd9591c9c3e3c00cf8c8284f8a 100644
--- a/runtime/bin/process_win.cc
+++ b/runtime/bin/process_win.cc
@@ -68,8 +68,11 @@ class ProcessInfo {
class ProcessInfoList {
public:
static void AddProcess(DWORD pid, HANDLE handle, HANDLE pipe) {
- // Create a wait operation for the process handle to extract
- // the exit code.
+ // Register a callback to extract the exit code, when the process
+ // is signaled. The callback runs in a independent thread from the OS pool.
+ // Because the callback depends on the process list containing
+ // the process, lock the mutex until the process is added to the list.
+ MutexLocker locker(&mutex_);
HANDLE wait_handle = INVALID_HANDLE_VALUE;
BOOL success = RegisterWaitForSingleObject(
&wait_handle,
@@ -82,8 +85,7 @@ class ProcessInfoList {
FATAL("Failed to register exit code wait operation.");
}
ProcessInfo* info = new ProcessInfo(pid, handle, wait_handle, pipe);
- // Now mutate the process list under the mutex.
- MutexLocker locker(&mutex_);
+ // Mutate the process list under the mutex.
info->set_next(active_processes_);
active_processes_ = info;
}
@@ -509,7 +511,9 @@ bool Process::Kill(intptr_t id, int signal) {
&process_handle,
&wait_handle,
&exit_pipe);
- ASSERT(success);
+ if (!success) {
+ return true; // The process has already died. Report a successful kill.
Anders Johnsen 2012/08/21 14:33:47 Returning true here is inconsistent with linux/mac
+ }
BOOL result = TerminateProcess(process_handle, -1);
if (!result) {
return false;
« 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