ScopedProcessInformation protects against process/thread handle leaks from CreateProcess calls.

base/win/scoped_process_information.cc

Index: base/win/scoped_process_information.cc
diff --git a/base/win/scoped_process_information.cc b/base/win/scoped_process_information.cc
new file mode 100644
index 0000000000000000000000000000000000000000..4659de1c1aaa495461e45fcf2627e6c2d64cff3e
--- /dev/null
+++ b/base/win/scoped_process_information.cc
@@ -0,0 +1,66 @@
+// Copyright (c) 2012 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.
+
+#include "base/win/scoped_process_information.h"
+
+#include "base/logging.h"
+
+namespace base {
+namespace win {
+
+ScopedProcessInformation::ScopedProcessInformation()
+    : process_information_() {
+}
+
+ScopedProcessInformation::~ScopedProcessInformation() {
+  Close();
+}
+
+PROCESS_INFORMATION* ScopedProcessInformation::Receive() {
+  DCHECK(!IsValid()) << "process_information_ must be NULL";
+  return &process_information_;
+}
+
+bool ScopedProcessInformation::IsValid() const {
+  return process_information_.hThread || process_information_.hProcess ||
+    process_information_.dwProcessId || process_information_.dwThreadId;

[alexeypa (please no reviews), 2012/03/29 04:51:35]

I think that it should test only handles but not the IDs. However you might find
it useful to DCHECK that both handle and ID are either null or not null. 

[erikwright (departed), 2012/03/30 17:30:27]

The only way for handle and id to not be in the same state is if someone does
*pi->Receive()->dwProcessId = X without also setting hProcess.

That's sufficiently unexpected that I'd prefer to maintain the otherwise
internal consistency that id and handle are set/checked/cleared/taken together.

[alexeypa (please no reviews), 2012/03/30 18:26:39]

There is one example in the accompanying CL (the sandbox changes) that modified
hProcess without touching dwProcessId. I don't think it is a problem. It just
would be nice it we had some protection against such an inconsistency.

+}
+
+void ScopedProcessInformation::Close() {
+  if (process_information_.hThread)
+    ::CloseHandle(process_information_.hThread);

[alexeypa (please no reviews), 2012/03/29 04:51:35]

It will be useful for debugging if NOTREACHED() will be invoked if CloseHandle
returns an error.

PS. There is such a check in ntdll.dll, but, I believe, it is not always active.

[erikwright (departed), 2012/03/30 17:30:27]

Done.

+  if (process_information_.hProcess)
+    ::CloseHandle(process_information_.hProcess);
+  Reset();
+}
+
+PROCESS_INFORMATION ScopedProcessInformation::Take() {

[alexeypa (please no reviews), 2012/03/29 04:51:35]

Now, since ScopedProcessInformation is not limited by GenericScopedHandle
interface it may be better to use Swap() semantic. I.e. somethink like:

void Swap(ScopedProcessInformation& other) {
  <swap_process_handle>;
  <swap_thred_handle>;
}

It surely depends on how Take() is used in the code and whether Take is more
convenient. Certainly ignore this comment if Take() is more convenient.

[erikwright (departed), 2012/03/30 17:30:27]

Looking through your suggestions on the use of Swap led me to learn more about
sandbox/ than I had ever wanted, but it was a good suggestion.

For the moment I've left Take() here, even though I don't have an immediate
client for it, because it seems correct from a symmetry point of view. WDYT?

[alexeypa (please no reviews), 2012/03/30 18:26:39]

The main reason to get rid of Take() is to reduce exposure of the naked
PROCESS_INFORMATION structure. Without Take there will be only Receive() and
individual HANDLE accessors (which seem to be safer because having a naked
HANDLE makes people think how to close it, while having a naked
PROCESS_INFORMATION structure - not so much).

+  PROCESS_INFORMATION process_information = process_information_;
+  Reset();
+  return process_information;
+}
+
+HANDLE ScopedProcessInformation::TakeProcessHandle() {
+  HANDLE process = process_information_.hProcess;
+  process_information_.hProcess = NULL;
+  process_information_.dwProcessId = 0;
+  return process;
+}
+
+HANDLE ScopedProcessInformation::TakeThreadHandle() {
+  HANDLE thread = process_information_.hThread;
+  process_information_.hThread = NULL;
+  process_information_.dwThreadId = 0;
+  return thread;
+}
+
+void ScopedProcessInformation::Reset() {
+  process_information_.hThread = NULL;
+  process_information_.hProcess = NULL;
+  process_information_.dwProcessId = 0;
+  process_information_.dwThreadId = 0;
+}
+
+}  // namespace win
+}  // namespace base