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 .
This commit is contained in:
Scott Graham 2015-09-11 15:34:35 -07:00
parent ccf9f98519
commit 81269ee676
2 changed files with 14 additions and 10 deletions

View File

@ -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();

View File

@ -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),