Enforce handle ownership in base::Process

base/process/process_win.cc

Index: base/process/process_win.cc
diff --git a/base/process/process_win.cc b/base/process/process_win.cc
index 1217b509896b3a015f7636c94734416d469b4641..05041b20ddaa7e8f908acc242dce2e45df066046 100644
--- a/base/process/process_win.cc
+++ b/base/process/process_win.cc
@@ -10,20 +10,83 @@
 
 namespace base {
 
+Process::Process(ProcessHandle handle)
+    : is_current_process_(false),
+      process_(handle) {
+  CHECK_NE(handle, ::GetCurrentProcess());
+}
+
+Process::Process(RValue other)
+    : is_current_process_(other.object->is_current_process_),
+      process_(other.object->process_.Take()) {
+  other.object->Close();
+}
+
+Process& Process::operator=(RValue other) {
+  if (this != other.object) {
+    process_.Set(other.object->process_.Take());
+    is_current_process_ = other.object->is_current_process_;
+    other.object->Close();
+  }
+  return *this;
+}
+
+// static
+Process Process::Current() {
+  Process process;
+  process.is_current_process_ = true;
+  return process.Pass();
+}
+
+// static
+bool Process::CanBackgroundProcesses() {
+  return true;
+}
+
+bool Process::IsValid() const {
+  return process_.IsValid() || is_current();
+}
+
+ProcessHandle Process::Handle() const {
+  return is_current_process_ ? GetCurrentProcess() : process_.Get();
+}
+
+Process Process::Duplicate() const {
+  if (is_current())
+    return Current();
+
+  ProcessHandle out_handle;
+  if (!IsValid() || !::DuplicateHandle(GetCurrentProcess(),
+                                       Handle(),
+                                       GetCurrentProcess(),
+                                       &out_handle,
+                                       0,
+                                       FALSE,
+                                       DUPLICATE_SAME_ACCESS)) {
+    return Process();
+  }
+  return Process(out_handle);
+}
+
+ProcessId Process::pid() const {
+  DCHECK(IsValid());
+  return GetProcId(Handle());
+}
+
+bool Process::is_current() const {
+  return is_current_process_;
+}
+
 void Process::Close() {
-  if (!process_)
+  is_current_process_ = false;
+  if (!process_.IsValid())
     return;
 
-  // Don't call CloseHandle on a pseudo-handle.
-  if (process_ != ::GetCurrentProcess())
-    ::CloseHandle(process_);
-
-  process_ = NULL;
+  process_.Close();
 }
 
 void Process::Terminate(int result_code) {
-  if (!process_)
-    return;
+  DCHECK(IsValid());

[Lei Zhang, 2014/10/16 02:50:08]

Can you give some reassurances that these DCHECKs won't fail?

[rvargas (doing something else), 2014/10/16 19:06:10]

The DCHECKs are going through internal state only so they do not depend on
anything that for example the OS would do (like kill a process).

The intention is for them to fire as soon as someone misuses the method... like
trying to terminate a process that was never created properly in the first
place.

 
   // Call NtTerminateProcess directly, without going through the import table,
   // which might have been hooked with a buggy replacement by third party
@@ -32,12 +95,11 @@ void Process::Terminate(int result_code) {
   typedef UINT (WINAPI *TerminateProcessPtr)(HANDLE handle, UINT code);
   TerminateProcessPtr terminate_process = reinterpret_cast<TerminateProcessPtr>(
       GetProcAddress(module, "NtTerminateProcess"));
-  terminate_process(process_, result_code);
+  terminate_process(Handle(), result_code);
 }
 
 bool Process::IsProcessBackgrounded() const {
-  if (!process_)
-    return false;  // Failure case.
+  DCHECK(IsValid());
   DWORD priority = GetPriority();
   if (priority == 0)
     return false;  // Failure case.
@@ -46,47 +108,24 @@ bool Process::IsProcessBackgrounded() const {
 }
 
 bool Process::SetProcessBackgrounded(bool value) {
-  if (!process_)
-    return false;
+  DCHECK(IsValid());
   // Vista and above introduce a real background mode, which not only
   // sets the priority class on the threads but also on the IO generated
   // by it. Unfortunately it can only be set for the calling process.
   DWORD priority;
-  if ((base::win::GetVersion() >= base::win::VERSION_VISTA) &&
-      (process_ == ::GetCurrentProcess())) {
+  if ((base::win::GetVersion() >= base::win::VERSION_VISTA) && (is_current())) {
     priority = value ? PROCESS_MODE_BACKGROUND_BEGIN :
                        PROCESS_MODE_BACKGROUND_END;
   } else {
     priority = value ? BELOW_NORMAL_PRIORITY_CLASS : NORMAL_PRIORITY_CLASS;
   }
 
-  return (::SetPriorityClass(process_, priority) != 0);
-}
-
-ProcessId Process::pid() const {
-  if (process_ == 0)
-    return 0;
-
-  return GetProcId(process_);
-}
-
-bool Process::is_current() const {
-  return process_ == GetCurrentProcess();
-}
-
-// static
-Process Process::Current() {
-  return Process(::GetCurrentProcess());
-}
-
-// static
-bool Process::CanBackgroundProcesses() {
-  return true;
+  return (::SetPriorityClass(Handle(), priority) != 0);
 }
 
 int Process::GetPriority() const {
-  DCHECK(process_);
-  return ::GetPriorityClass(process_);
+  DCHECK(IsValid());
+  return ::GetPriorityClass(Handle());
 }
 
 }  // namespace base