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

base/win/scoped_process_information.h

Index: base/win/scoped_process_information.h
diff --git a/base/win/scoped_process_information.h b/base/win/scoped_process_information.h
new file mode 100644
index 0000000000000000000000000000000000000000..711b0cc71d4259ee7049887a73d5da2c62ab8c49
--- /dev/null
+++ b/base/win/scoped_process_information.h
@@ -0,0 +1,85 @@
+// 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.
+
+#ifndef BASE_WIN_SCOPED_PROCESS_INFORMATION_H_
+#define BASE_WIN_SCOPED_PROCESS_INFORMATION_H_
+#pragma once
+
+#include <windows.h>
+
+#include "base/win/scoped_handle.h"
+
+namespace base {
+namespace win {
+
+class ProcessInfoTraits {
+ public:
+  typedef PROCESS_INFORMATION Handle;
+
+  // Releases the PROCESS_INFORMATION's process and thread handles.
+  static bool CloseHandle(PROCESS_INFORMATION handle) {

[alexeypa (please no reviews), 2012/03/15 02:49:16]

You really want "PROCESS_INFORMATION& handle" here.

[erikwright (departed), 2012/03/15 03:29:17]

Well, the GenericScopedHandle is not expecting this method to modify the passed
Handle (c.f. the existing implementation that passes a HANDLE by value). But I
do agree that here and elsewhere, const references are correct.

+    bool ret = true;
+    if (handle.hThread && !::CloseHandle(handle.hThread))
+      ret = false;
+    if (handle.hProcess && !::CloseHandle(handle.hProcess))
+      ret = false;
+    handle = NullHandle();

[alexeypa (please no reviews), 2012/03/15 02:49:16]

|handle| is a local copy on the stack. This does not change the copy of the
structure contained by GenericScopedHandle.

[erikwright (departed), 2012/03/15 03:29:17]

Done.

+    return ret;
+  }
+
+  static bool IsHandleValid(PROCESS_INFORMATION handle) {

[alexeypa (please no reviews), 2012/03/15 02:49:16]

const PROCESS_INFORMATION&

[erikwright (departed), 2012/03/15 03:29:17]

Done.

+    return handle.hThread || handle.hProcess ||
+      handle.dwProcessId || handle.dwThreadId;
+  }
+
+  static bool IsSame(PROCESS_INFORMATION lhs, PROCESS_INFORMATION rhs) {

[alexeypa (please no reviews), 2012/03/15 02:49:16]

const PROCESS_INFORMATION&

[erikwright (departed), 2012/03/15 03:29:17]

Done.

+    return lhs.hProcess == rhs.hProcess &&
+      lhs.hThread == rhs.hThread &&
+      lhs.dwProcessId == rhs.dwProcessId &&
+      lhs.dwThreadId == rhs.dwThreadId;
+  }
+
+  static PROCESS_INFORMATION NullHandle() {

[alexeypa (please no reviews), 2012/03/15 02:49:16]

I don't think returning raw structures like this is the best practice. It is
legal, but not efficient. 

It is better to wrap a pointer to PROCESS_INFORMATION into GenericScopedHandle.

[erikwright (departed), 2012/03/15 03:29:17]

Hmm. Then the client needs to have both a PROCESS_INFORMATION on the stack and a
ScopedProcessInformation (or a ScopedProcessInformationReleaser, or something).

In general I expect the call to NullHandle to be wiped out during compilation of
the templates, but here I've offered an approach with a const reference and a
static null value. WDYT?

[alexeypa (please no reviews), 2012/03/15 16:38:27]

After a second thought, wrapping a pointer does not look like a such a good
idea. The code will not be very readable.
I think this will work just fine. This should eliminate unnecessary copying of
the structure in the stack.

+    PROCESS_INFORMATION null_process_information = {0};
+    return null_process_information;
+  }
+};
+
+// Manages the closing of process and thread handles from PROCESS_INFORMATION
+// structures. Allows clients to take ownership of either handle independently.
+class ScopedProcessInformation : public GenericScopedHandle<ProcessInfoTraits> {

[grt (UTC plus 2), 2012/03/15 02:59:30]

did you consider something like this:

class ScopedProcessInformation : public PROCESS_INFORMATION {
  ScopedProcessInformation() : hProcess(), hThread() { }
  ~ScopedProcessInformation() {
    if (hProcess)
      ::CloseHandle(hProcess);
    if (hTread)
      ::CloseHandle(hThread);
  }
  HANDLE TakeProcessHandle() { HANDLE p = hProcess; hProcess = NULL; return p; }
  HANDLE TakeThread() { ... }
  // etc...
};

then consumers don't need calls to Receive or Get, they just treat it like a
normal PROCESS_INFORMATION.  WDYT?

[erikwright (departed), 2012/03/15 03:29:17]

WRT Get(), it could also just be process_handle() and thread_handle(), which is
not dramatically more than .hProcess and hThread.

Receive() has an additional protection which is that it requires the object to
currently be NULL. With public derivation of PROCESS_INFORMATION, you could do,
for example:

ScopedProcessInformation pi;
CreateProcess(..., &pi);
CreateProcess(..., &pi);

The first pi is lost. So this would really not be a ScopedX on par with all
other scoped_.<x> and ScopedX classes in Chromium.

[grt (UTC plus 2), 2012/03/15 18:01:09]

It might be a can of worms, but you could:
Handle* operator&() { ... }

I'll bow out now.

+ public:
+  ScopedProcessInformation()
+      : GenericScopedHandle(ProcessInfoTraits::NullHandle()) {}
+
+  explicit ScopedProcessInformation(Handle handle)
+      : GenericScopedHandle(handle) {}
+
+  // Transfers ownership of the process handle away from this object. The
+  // hProcess and dwProcessId members will be reset.
+  HANDLE TakeProcessHandle() {
+    PROCESS_INFORMATION process_info = Take();
+    HANDLE process = process_info.hProcess;
+    process_info.hProcess = NULL;
+    process_info.dwProcessId = 0;
+    Set(process_info);

[alexeypa (please no reviews), 2012/03/15 16:38:27]

I would just modified the structure directly here avoiding the overhead of
Take() and Set().

[erikwright (departed), 2012/03/15 18:02:44]

It's a private member. I'd rather not make it protected, so this mildly
round-about approach seems OK.

+    return process;
+  }
+
+  // Transfers ownership of the thread handle away from this object. The hThread
+  // and dwThreadId members will be reset.
+  HANDLE TakeThreadHandle() {
+    PROCESS_INFORMATION process_info = Take();
+    HANDLE thread = process_info.hThread;
+    process_info.hThread = NULL;
+    process_info.dwThreadId = 0;

[alexeypa (please no reviews), 2012/03/15 16:38:27]

Same as above.

+    Set(process_info);
+    return thread;
+  }
+};
+
+}  // namespace win
+}  // namespace base
+
+#endif  // BASE_SCOPED_HANDLE_WIN_H_