From 81269ee676d8aa5f7e5e7f3a142de6718293de6d Mon Sep 17 00:00:00 2001
From: Scott Graham <scottmg@chromium.org>
Date: Fri, 11 Sep 2015 15:34:35 -0700
Subject: [PATCH] win: Fix pipe leak on connection

The pipe handle was being leaked on connections (oops!). On XP this
resulted in the next test's CreateNamedPipe to fail, because the
previous one still existed (because all handles were not closed). More
recent OSs are more forgiving so I got away with the buggy code.

R=mark@chromium.org
BUG=crashpad:50

Review URL: https://codereview.chromium.org/1337953003 .
---
 util/win/exception_handler_server.cc  |  2 ++
 util/win/registration_protocol_win.cc | 22 ++++++++++++----------
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc
index 2d702f20..50a5fa3c 100644
--- a/util/win/exception_handler_server.cc
+++ b/util/win/exception_handler_server.cc
@@ -211,6 +211,7 @@ void ExceptionHandlerServer::Run(Delegate* delegate,
                         512,
                         0,
                         nullptr);
+    PCHECK(pipe != INVALID_HANDLE_VALUE) << "CreateNamedPipe";
 
     // Ownership of this object (and the pipe instance) is given to the new
     // thread. We close the thread handles at the end of the scope. They clean
@@ -224,6 +225,7 @@ void ExceptionHandlerServer::Run(Delegate* delegate,
                                          shutdown_token);
     thread_handles[i].reset(
         CreateThread(nullptr, 0, &PipeServiceProc, context, 0, nullptr));
+    PCHECK(thread_handles[i].is_valid()) << "CreateThread";
   }
 
   delegate->ExceptionHandlerServerStarted();
diff --git a/util/win/registration_protocol_win.cc b/util/win/registration_protocol_win.cc
index a4a3c1c8..3e0fdf21 100644
--- a/util/win/registration_protocol_win.cc
+++ b/util/win/registration_protocol_win.cc
@@ -17,6 +17,7 @@
 #include <windows.h>
 
 #include "base/logging.h"
+#include "util/win/scoped_handle.h"
 
 namespace crashpad {
 
@@ -25,26 +26,27 @@ bool SendToCrashHandlerServer(const base::string16& pipe_name,
                               crashpad::ServerToClientMessage* response) {
   int tries = 5;
   while (tries > 0) {
-    HANDLE pipe = CreateFile(pipe_name.c_str(),
-                             GENERIC_READ | GENERIC_WRITE,
-                             0,
-                             nullptr,
-                             OPEN_EXISTING,
-                             SECURITY_SQOS_PRESENT | SECURITY_IDENTIFICATION,
-                             nullptr);
-    if (pipe == INVALID_HANDLE_VALUE) {
+    ScopedFileHANDLE pipe(
+        CreateFile(pipe_name.c_str(),
+                   GENERIC_READ | GENERIC_WRITE,
+                   0,
+                   nullptr,
+                   OPEN_EXISTING,
+                   SECURITY_SQOS_PRESENT | SECURITY_IDENTIFICATION,
+                   nullptr));
+    if (!pipe.is_valid()) {
       Sleep(10);
       --tries;
       continue;
     }
     DWORD mode = PIPE_READMODE_MESSAGE;
-    if (!SetNamedPipeHandleState(pipe, &mode, nullptr, nullptr)) {
+    if (!SetNamedPipeHandleState(pipe.get(), &mode, nullptr, nullptr)) {
       PLOG(ERROR) << "SetNamedPipeHandleState";
       return false;
     }
     DWORD bytes_read = 0;
     BOOL result = TransactNamedPipe(
-        pipe,
+        pipe.get(),
         // This is [in], but is incorrectly declared non-const.
         const_cast<crashpad::ClientToServerMessage*>(&message),
         sizeof(message),