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

Side by Side Diff: base/process_util_win.cc

Issue 9700038: ScopedProcessInformation protects against process/thread handle leaks from CreateProcess calls. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Add tests for new functions. Created 8 years, 8 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 unified diff | Download patch | Annotate | Revision Log
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "base/process_util.h" 5 #include "base/process_util.h"
6 6
7 #include <fcntl.h> 7 #include <fcntl.h>
8 #include <io.h> 8 #include <io.h>
9 #include <windows.h> 9 #include <windows.h>
10 #include <userenv.h> 10 #include <userenv.h>
11 #include <psapi.h> 11 #include <psapi.h>
12 12
13 #include <ios> 13 #include <ios>
14 14
15 #include "base/bind.h" 15 #include "base/bind.h"
16 #include "base/bind_helpers.h" 16 #include "base/bind_helpers.h"
17 #include "base/command_line.h" 17 #include "base/command_line.h"
18 #include "base/debug/stack_trace.h" 18 #include "base/debug/stack_trace.h"
19 #include "base/logging.h" 19 #include "base/logging.h"
20 #include "base/memory/scoped_ptr.h" 20 #include "base/memory/scoped_ptr.h"
21 #include "base/message_loop.h" 21 #include "base/message_loop.h"
22 #include "base/metrics/histogram.h" 22 #include "base/metrics/histogram.h"
23 #include "base/sys_info.h" 23 #include "base/sys_info.h"
24 #include "base/win/object_watcher.h" 24 #include "base/win/object_watcher.h"
25 #include "base/win/scoped_handle.h" 25 #include "base/win/scoped_handle.h"
26 #include "base/win/scoped_process_information.h"
26 #include "base/win/windows_version.h" 27 #include "base/win/windows_version.h"
27 28
28 // userenv.dll is required for CreateEnvironmentBlock(). 29 // userenv.dll is required for CreateEnvironmentBlock().
29 #pragma comment(lib, "userenv.lib") 30 #pragma comment(lib, "userenv.lib")
30 31
31 namespace base { 32 namespace base {
32 33
33 namespace { 34 namespace {
34 35
35 // System pagesize. This value remains constant on x86/64 architectures. 36 // System pagesize. This value remains constant on x86/64 architectures.
(...skipping 263 matching lines...) Expand 10 before | Expand all | Expand 10 after
299 300
300 bool LaunchProcess(const string16& cmdline, 301 bool LaunchProcess(const string16& cmdline,
301 const LaunchOptions& options, 302 const LaunchOptions& options,
302 ProcessHandle* process_handle) { 303 ProcessHandle* process_handle) {
303 STARTUPINFO startup_info = {}; 304 STARTUPINFO startup_info = {};
304 startup_info.cb = sizeof(startup_info); 305 startup_info.cb = sizeof(startup_info);
305 if (options.empty_desktop_name) 306 if (options.empty_desktop_name)
306 startup_info.lpDesktop = L""; 307 startup_info.lpDesktop = L"";
307 startup_info.dwFlags = STARTF_USESHOWWINDOW; 308 startup_info.dwFlags = STARTF_USESHOWWINDOW;
308 startup_info.wShowWindow = options.start_hidden ? SW_HIDE : SW_SHOW; 309 startup_info.wShowWindow = options.start_hidden ? SW_HIDE : SW_SHOW;
309 PROCESS_INFORMATION process_info;
310 310
311 DWORD flags = 0; 311 DWORD flags = 0;
312 312
313 if (options.job_handle) { 313 if (options.job_handle) {
314 flags |= CREATE_SUSPENDED; 314 flags |= CREATE_SUSPENDED;
315 315
316 // If this code is run under a debugger, the launched process is 316 // If this code is run under a debugger, the launched process is
317 // automatically associated with a job object created by the debugger. 317 // automatically associated with a job object created by the debugger.
318 // The CREATE_BREAKAWAY_FROM_JOB flag is used to prevent this. 318 // The CREATE_BREAKAWAY_FROM_JOB flag is used to prevent this.
319 flags |= CREATE_BREAKAWAY_FROM_JOB; 319 flags |= CREATE_BREAKAWAY_FROM_JOB;
320 } 320 }
321 321
322 base::win::ScopedProcessInformation process_info;
323
322 if (options.as_user) { 324 if (options.as_user) {
323 flags |= CREATE_UNICODE_ENVIRONMENT; 325 flags |= CREATE_UNICODE_ENVIRONMENT;
324 void* enviroment_block = NULL; 326 void* enviroment_block = NULL;
325 327
326 if (!CreateEnvironmentBlock(&enviroment_block, options.as_user, FALSE)) 328 if (!CreateEnvironmentBlock(&enviroment_block, options.as_user, FALSE))
327 return false; 329 return false;
328 330
329 BOOL launched = 331 BOOL launched =
330 CreateProcessAsUser(options.as_user, NULL, 332 CreateProcessAsUser(options.as_user, NULL,
331 const_cast<wchar_t*>(cmdline.c_str()), 333 const_cast<wchar_t*>(cmdline.c_str()),
332 NULL, NULL, options.inherit_handles, flags, 334 NULL, NULL, options.inherit_handles, flags,
333 enviroment_block, NULL, &startup_info, 335 enviroment_block, NULL, &startup_info,
334 &process_info); 336 process_info.Receive());
335 DestroyEnvironmentBlock(enviroment_block); 337 DestroyEnvironmentBlock(enviroment_block);
336 if (!launched) 338 if (!launched)
337 return false; 339 return false;
338 } else { 340 } else {
339 if (!CreateProcess(NULL, 341 if (!CreateProcess(NULL,
340 const_cast<wchar_t*>(cmdline.c_str()), NULL, NULL, 342 const_cast<wchar_t*>(cmdline.c_str()), NULL, NULL,
341 options.inherit_handles, flags, NULL, NULL, 343 options.inherit_handles, flags, NULL, NULL,
342 &startup_info, &process_info)) { 344 &startup_info, process_info.Receive())) {
343 return false; 345 return false;
344 } 346 }
345 } 347 }
346 348
347 if (options.job_handle) { 349 if (options.job_handle) {
348 if (0 == AssignProcessToJobObject(options.job_handle, 350 if (0 == AssignProcessToJobObject(options.job_handle,
349 process_info.hProcess)) { 351 process_info.process_handle())) {
350 DLOG(ERROR) << "Could not AssignProcessToObject."; 352 DLOG(ERROR) << "Could not AssignProcessToObject.";
351 KillProcess(process_info.hProcess, kProcessKilledExitCode, true); 353 KillProcess(process_info.process_handle(), kProcessKilledExitCode, true);
352 CloseHandle(process_info.hProcess);
353 CloseHandle(process_info.hThread);
354 return false; 354 return false;
355 } 355 }
356 356
357 ResumeThread(process_info.hThread); 357 ResumeThread(process_info.thread_handle());
358 } 358 }
359 359
360 // Handles must be closed or they will leak.
361 CloseHandle(process_info.hThread);
362
363 if (options.wait) 360 if (options.wait)
364 WaitForSingleObject(process_info.hProcess, INFINITE); 361 WaitForSingleObject(process_info.process_handle(), INFINITE);
365 362
366 // If the caller wants the process handle, we won't close it. 363 // If the caller wants the process handle, we won't close it.
367 if (process_handle) { 364 if (process_handle)
368 *process_handle = process_info.hProcess; 365 *process_handle = process_info.TakeProcessHandle();
369 } else { 366
370 CloseHandle(process_info.hProcess);
371 }
372 return true; 367 return true;
373 } 368 }
374 369
375 bool LaunchProcess(const CommandLine& cmdline, 370 bool LaunchProcess(const CommandLine& cmdline,
376 const LaunchOptions& options, 371 const LaunchOptions& options,
377 ProcessHandle* process_handle) { 372 ProcessHandle* process_handle) {
378 return LaunchProcess(cmdline.GetCommandLineString(), options, process_handle); 373 return LaunchProcess(cmdline.GetCommandLineString(), options, process_handle);
379 } 374 }
380 375
381 bool SetJobObjectAsKillOnJobClose(HANDLE job_object) { 376 bool SetJobObjectAsKillOnJobClose(HANDLE job_object) {
(...skipping 45 matching lines...) Expand 10 before | Expand all | Expand 10 after
427 win::ScopedHandle scoped_out_write(out_write); 422 win::ScopedHandle scoped_out_write(out_write);
428 423
429 // Ensure the read handle to the pipe for STDOUT is not inherited. 424 // Ensure the read handle to the pipe for STDOUT is not inherited.
430 if (!SetHandleInformation(out_read, HANDLE_FLAG_INHERIT, 0)) { 425 if (!SetHandleInformation(out_read, HANDLE_FLAG_INHERIT, 0)) {
431 NOTREACHED() << "Failed to disabled pipe inheritance"; 426 NOTREACHED() << "Failed to disabled pipe inheritance";
432 return false; 427 return false;
433 } 428 }
434 429
435 std::wstring writable_command_line_string(cl.GetCommandLineString()); 430 std::wstring writable_command_line_string(cl.GetCommandLineString());
436 431
437 PROCESS_INFORMATION proc_info = { 0 }; 432 base::win::ScopedProcessInformation proc_info;
438 STARTUPINFO start_info = { 0 }; 433 STARTUPINFO start_info = { 0 };
439 434
440 start_info.cb = sizeof(STARTUPINFO); 435 start_info.cb = sizeof(STARTUPINFO);
441 start_info.hStdOutput = out_write; 436 start_info.hStdOutput = out_write;
442 // Keep the normal stdin and stderr. 437 // Keep the normal stdin and stderr.
443 start_info.hStdInput = GetStdHandle(STD_INPUT_HANDLE); 438 start_info.hStdInput = GetStdHandle(STD_INPUT_HANDLE);
444 start_info.hStdError = GetStdHandle(STD_ERROR_HANDLE); 439 start_info.hStdError = GetStdHandle(STD_ERROR_HANDLE);
445 start_info.dwFlags |= STARTF_USESTDHANDLES; 440 start_info.dwFlags |= STARTF_USESTDHANDLES;
446 441
447 // Create the child process. 442 // Create the child process.
448 if (!CreateProcess(NULL, 443 if (!CreateProcess(NULL,
449 &writable_command_line_string[0], 444 &writable_command_line_string[0],
450 NULL, NULL, 445 NULL, NULL,
451 TRUE, // Handles are inherited. 446 TRUE, // Handles are inherited.
452 0, NULL, NULL, &start_info, &proc_info)) { 447 0, NULL, NULL, &start_info, proc_info.Receive())) {
453 NOTREACHED() << "Failed to start process"; 448 NOTREACHED() << "Failed to start process";
454 return false; 449 return false;
455 } 450 }
456 451
457 // We don't need the thread handle, close it now.
458 CloseHandle(proc_info.hThread);
459
460 // Close our writing end of pipe now. Otherwise later read would not be able 452 // Close our writing end of pipe now. Otherwise later read would not be able
461 // to detect end of child's output. 453 // to detect end of child's output.
462 scoped_out_write.Close(); 454 scoped_out_write.Close();
463 455
464 // Read output from the child process's pipe for STDOUT 456 // Read output from the child process's pipe for STDOUT
465 const int kBufferSize = 1024; 457 const int kBufferSize = 1024;
466 char buffer[kBufferSize]; 458 char buffer[kBufferSize];
467 459
468 for (;;) { 460 for (;;) {
469 DWORD bytes_read = 0; 461 DWORD bytes_read = 0;
470 BOOL success = ReadFile(out_read, buffer, kBufferSize, &bytes_read, NULL); 462 BOOL success = ReadFile(out_read, buffer, kBufferSize, &bytes_read, NULL);
471 if (!success || bytes_read == 0) 463 if (!success || bytes_read == 0)
472 break; 464 break;
473 output->append(buffer, bytes_read); 465 output->append(buffer, bytes_read);
474 } 466 }
475 467
476 // Let's wait for the process to finish. 468 // Let's wait for the process to finish.
477 WaitForSingleObject(proc_info.hProcess, INFINITE); 469 WaitForSingleObject(proc_info.process_handle(), INFINITE);
478 CloseHandle(proc_info.hProcess);
479 470
480 return true; 471 return true;
481 } 472 }
482 473
483 bool KillProcess(ProcessHandle process, int exit_code, bool wait) { 474 bool KillProcess(ProcessHandle process, int exit_code, bool wait) {
484 bool result = (TerminateProcess(process, exit_code) != FALSE); 475 bool result = (TerminateProcess(process, exit_code) != FALSE);
485 if (result && wait) { 476 if (result && wait) {
486 // The process may not end immediately due to pending I/O 477 // The process may not end immediately due to pending I/O
487 if (WAIT_OBJECT_0 != WaitForSingleObject(process, 60 * 1000)) 478 if (WAIT_OBJECT_0 != WaitForSingleObject(process, 60 * 1000))
488 DLOG(ERROR) << "Error waiting for process exit: " << GetLastError(); 479 DLOG(ERROR) << "Error waiting for process exit: " << GetLastError();
(...skipping 515 matching lines...) Expand 10 before | Expand all | Expand 10 after
1004 995
1005 PERFORMANCE_INFORMATION info; 996 PERFORMANCE_INFORMATION info;
1006 if (!InternalGetPerformanceInfo(&info, sizeof(info))) { 997 if (!InternalGetPerformanceInfo(&info, sizeof(info))) {
1007 DLOG(ERROR) << "Failed to fetch internal performance info."; 998 DLOG(ERROR) << "Failed to fetch internal performance info.";
1008 return 0; 999 return 0;
1009 } 1000 }
1010 return (info.CommitTotal * system_info.dwPageSize) / 1024; 1001 return (info.CommitTotal * system_info.dwPageSize) / 1024;
1011 } 1002 }
1012 1003
1013 } // namespace base 1004 } // namespace base
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698