win: crashpad_handler should create its own pipe name in ephemeral mode

Allowing the client to create its own pipe name string caused a race
between client and server. Instead, in this mode, the server now creates
the pipe name along with a pipe, and returns it to its client via a
--handshake-handle. This guarantees that by the time the client gets the
pipe name, the server has already created it.

Ephemeral mode is now implied by --handshake-handle. The --persistent
option is gone. --persistent mode is enabled when using --pipe-name.

BUG=crashpad:69
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1432563003 .

client/crashpad_client_win.cc

@@ -19,7 +19,6 @@
 
 #include "base/atomicops.h"
 #include "base/logging.h"
-#include "base/rand_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
@@ -130,12 +129,21 @@ bool CrashpadClient::StartHandler(
     bool restartable) {
   DCHECK(ipc_pipe_.empty());
 
-  std::string ipc_pipe =
+  HANDLE pipe_read;
-      base::StringPrintf("\\\\.\\pipe\\crashpad_%d_", GetCurrentProcessId());
+  HANDLE pipe_write;
-  for (int index = 0; index < 16; ++index) {
+  SECURITY_ATTRIBUTES security_attributes = {};
-    ipc_pipe.append(1, static_cast<char>(base::RandInt('A', 'Z')));
+  security_attributes.nLength = sizeof(security_attributes);
+  security_attributes.bInheritHandle = TRUE;
+  if (!CreatePipe(&pipe_read, &pipe_write, &security_attributes, 0)) {
+    PLOG(ERROR) << "CreatePipe";
+    return false;
   }
-  ipc_pipe_ = base::UTF8ToUTF16(ipc_pipe);
+  ScopedFileHandle pipe_read_owner(pipe_read);
+  ScopedFileHandle pipe_write_owner(pipe_write);
+
+  // The new process only needs the write side of the pipe.
+  BOOL rv = SetHandleInformation(pipe_read, HANDLE_FLAG_INHERIT, 0);
+  PLOG_IF(WARNING, !rv) << "SetHandleInformation";
 
   std::wstring command_line;
   AppendCommandLineArgument(handler.value(), &command_line);
@@ -158,8 +166,14 @@ bool CrashpadClient::StartHandler(
                              base::UTF8ToUTF16(kv.first + '=' + kv.second)),
         &command_line);
   }
-  AppendCommandLineArgument(FormatArgumentString("pipe-name", ipc_pipe_),
+
-                            &command_line);
+  // According to
+  // https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203, HANDLEs
+  // are always 32 bits.
+  AppendCommandLineArgument(
+      base::UTF8ToUTF16(base::StringPrintf("--handshake-handle=0x%x",
+                                           pipe_write)),
+      &command_line);
 
   STARTUPINFO startup_info = {};
   startup_info.cb = sizeof(startup_info);
@@ -168,16 +182,16 @@ bool CrashpadClient::StartHandler(
   startup_info.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE);
   startup_info.hStdError = GetStdHandle(STD_ERROR_HANDLE);
   PROCESS_INFORMATION process_info;
-  BOOL rv = CreateProcess(handler.value().c_str(),
+  rv = CreateProcess(handler.value().c_str(),
-                          &command_line[0],
+                     &command_line[0],
-                          nullptr,
+                     nullptr,
-                          nullptr,
+                     nullptr,
-                          true,
+                     true,
-                          0,
+                     0,
-                          nullptr,
+                     nullptr,
-                          nullptr,
+                     nullptr,
-                          &startup_info,
+                     &startup_info,
-                          &process_info);
+                     &process_info);
   if (!rv) {
     PLOG(ERROR) << "CreateProcess";
     return false;
@@ -189,6 +203,20 @@ bool CrashpadClient::StartHandler(
   rv = CloseHandle(process_info.hProcess);
   PLOG_IF(WARNING, !rv) << "CloseHandle process";
 
+  pipe_write_owner.reset();
+
+  uint32_t ipc_pipe_length;
+  if (!LoggingReadFile(pipe_read, &ipc_pipe_length, sizeof(ipc_pipe_length))) {
+    return false;
+  }
+
+  ipc_pipe_.resize(ipc_pipe_length);
+  if (ipc_pipe_length &&
+      !LoggingReadFile(
+          pipe_read, &ipc_pipe_[0], ipc_pipe_length * sizeof(ipc_pipe_[0]))) {
+    return false;
+  }
+
   return true;
 }
 

handler/crashpad_handler.ad

@@ -50,13 +50,15 @@ any upload in progress to complete. +SIGTERM+ is normally sent by launchd(8)
 when it determines that the server should exit.
 
 On Windows, clients register with this server by communicating with it via the
-named pipe identified by the *--pipe-name* argument. During registration, a
+named pipe identified by the *--pipe-name* argument. Alternatively, the server
-client provides the server with an OS event object that it will signal should it
+can create a new pipe with a random name and inform a client of this name via
-crash. The server obtains the client’s process handle and waits on the crash
+the *--handshake-handle* mechanism; clients may then register by communicating
-event object for a crash, as well as the client’s process handle for the client
+with it via that named pipe. During registration, a client provides the server
-to exit cleanly without crashing. When the server loses all clients and
+with an OS event object that it will signal should it crash. The server obtains
-*--persistent* is not specified, it exits after allowing any upload in progress
+the client’s process handle and waits on the crash event object for a crash, as
-to complete.
+well as the client’s process handle for the client to exit cleanly without
+crashing. When a server started via the *--handshake-handle* mechanism loses all
+of its clients, it exits after allowing any upload in progress to complete.
 
 It is not normally appropriate to invoke this program directly. Usually, it will
 be invoked by a Crashpad client using the Crashpad client library, or started by
@@ -105,16 +107,24 @@ tool is started by launchd(8) as a result of a message being sent to a service
 declared in a job’s +MachServices+ dictionary (see launchd.plist(5)). The
 service name may also be completely unknown to the system.
 
-*--persistent*::
+*--handshake-handle*='HANDLE'::
-Continue running after the last client exits. If this option is not specified,
+Perform the handshake with the initial client on the HANDLE at 'HANDLE'. Either
-this server will exit as soon as it has no clients, although on startup, it
+this option or *--pipe-name*, but not both, is required. This option is only
-always waits for at least one client to connect. This option is only valid on
+valid on Windows.
-Windows.
++
+When this option is present, the server creates a new named pipe at a random
+name and informs its client of the name. The server waits for at least one
+client to register, and exits when all clients have exited, after waiting for
+any uploads in progress to complete.
 
 *--pipe-name*='PIPE'::
 Listen on the given pipe name for connections from clients. 'PIPE' must be of
-the form +\\.\pipe\<somename>+. This option is required. This option is only
+the form +\\.\pipe\<somename>+. Either this option or *--handshake-handle*, but
-valid on Windows.
+not both, is required. This option is only valid on Windows.
++
+When this option is present, the server creates a named pipe at 'PIPE', a name
+known to both the server and its clients. The server continues running even
+after all clients have exited.
 
 *--reset-own-crash-exception-port-to-system-default*::
 Causes the exception handler server to set its own crash handler to the system

handler/main.cc

@@ -22,11 +22,13 @@
 #include "base/files/scoped_file.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "client/crash_report_database.h"
 #include "client/crashpad_client.h"
 #include "tools/tool_support.h"
 #include "handler/crash_report_upload_thread.h"
+#include "util/file/file_io.h"
 #include "util/stdlib/map_insert.h"
 #include "util/stdlib/string_number_conversion.h"
 #include "util/string/split_string.h"
@@ -62,7 +64,8 @@ void Usage(const base::FilePath& me) {
 "      --reset-own-crash-exception-port-to-system-default\n"
 "                              reset the server's exception handler to default\n"
 #elif defined(OS_WIN)
-"      --persistent            continue running after all clients exit\n"
+"      --handshake-handle=HANDLE\n"
+"                              create a new pipe and send its name via HANDLE\n"
 "      --pipe-name=PIPE        communicate with the client over PIPE\n"
 #endif  // OS_MACOSX
 "      --url=URL               send crash reports to this Breakpad server URL,\n"
@@ -88,7 +91,7 @@ int HandlerMain(int argc, char* argv[]) {
     kOptionMachService,
     kOptionResetOwnCrashExceptionPortToSystemDefault,
 #elif defined(OS_WIN)
-    kOptionPersistent,
+    kOptionHandshakeHandle,
     kOptionPipeName,
 #endif  // OS_MACOSX
     kOptionURL,
@@ -107,13 +110,14 @@ int HandlerMain(int argc, char* argv[]) {
     std::string mach_service;
     bool reset_own_crash_exception_port_to_system_default;
 #elif defined(OS_WIN)
-    bool persistent;
+    HANDLE handshake_handle;
     std::string pipe_name;
 #endif  // OS_MACOSX
   } options = {};
 #if defined(OS_MACOSX)
   options.handshake_fd = -1;
-  options.reset_own_crash_exception_port_to_system_default = false;
+#elif defined(OS_WIN)
+  options.handshake_handle = INVALID_HANDLE_VALUE;
 #endif
 
   const option long_options[] = {
@@ -127,7 +131,7 @@ int HandlerMain(int argc, char* argv[]) {
        nullptr,
        kOptionResetOwnCrashExceptionPortToSystemDefault},
 #elif defined(OS_WIN)
-      {"persistent", no_argument, nullptr, kOptionPersistent},
+      {"handshake-handle", required_argument, nullptr, kOptionHandshakeHandle},
       {"pipe-name", required_argument, nullptr, kOptionPipeName},
 #endif  // OS_MACOSX
       {"url", required_argument, nullptr, kOptionURL},
@@ -176,8 +180,19 @@ int HandlerMain(int argc, char* argv[]) {
         break;
       }
 #elif defined(OS_WIN)
-      case kOptionPersistent: {
+      case kOptionHandshakeHandle: {
-        options.persistent = true;
+        // According to
+        // https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203,
+        // HANDLEs are always 32 bits. This construction is used to read a full
+        // 32-bit number presented by the client, which uses 0x%x format, and
+        // sign-extend it.
+        unsigned int handle_uint;
+        if (!StringToNumber(optarg, &handle_uint) ||
+            (options.handshake_handle = reinterpret_cast<HANDLE>(
+                 static_cast<int>(handle_uint))) == INVALID_HANDLE_VALUE) {
+          ToolSupport::UsageHint(me, "--handshake-handle requires a HANDLE");
+          return EXIT_FAILURE;
+        }
         break;
       }
       case kOptionPipeName: {
@@ -217,11 +232,18 @@ int HandlerMain(int argc, char* argv[]) {
     return EXIT_FAILURE;
   }
 #elif defined(OS_WIN)
-  if (options.pipe_name.empty()) {
+  if (options.handshake_handle == INVALID_HANDLE_VALUE &&
-    ToolSupport::UsageHint(me, "--pipe-name is required");
+      options.pipe_name.empty()) {
+    ToolSupport::UsageHint(me, "--handshake-handle or --pipe-name is required");
     return EXIT_FAILURE;
   }
-#endif
+  if (options.handshake_handle != INVALID_HANDLE_VALUE &&
+      !options.pipe_name.empty()) {
+    ToolSupport::UsageHint(
+        me, "--handshake-handle and --pipe-name are incompatible");
+    return EXIT_FAILURE;
+  }
+#endif  // OS_MACOSX
 
   if (!options.database) {
     ToolSupport::UsageHint(me, "--database is required");
@@ -261,8 +283,26 @@ int HandlerMain(int argc, char* argv[]) {
   ExceptionHandlerServer exception_handler_server(
       receive_right.Pass(), !options.mach_service.empty());
 #elif defined(OS_WIN)
-  ExceptionHandlerServer exception_handler_server(options.pipe_name,
+  ExceptionHandlerServer exception_handler_server(!options.pipe_name.empty());
-                                                  options.persistent);
+
+  std::string pipe_name;
+  if (!options.pipe_name.empty()) {
+    exception_handler_server.SetPipeName(base::UTF8ToUTF16(options.pipe_name));
+  } else if (options.handshake_handle != INVALID_HANDLE_VALUE) {
+    std::wstring pipe_name = exception_handler_server.CreatePipe();
+
+    uint32_t pipe_name_length = static_cast<uint32_t>(pipe_name.size());
+    if (!LoggingWriteFile(options.handshake_handle,
+                          &pipe_name_length,
+                          sizeof(pipe_name_length))) {
+      return EXIT_FAILURE;
+    }
+    if (!LoggingWriteFile(options.handshake_handle,
+                          pipe_name.c_str(),
+                          pipe_name.size() * sizeof(pipe_name[0]))) {
+      return EXIT_FAILURE;
+    }
+  }
 #endif  // OS_MACOSX
 
   scoped_ptr<CrashReportDatabase> database(CrashReportDatabase::Initialize(

handler/win/crashy_test_program.cc

@@ -112,8 +112,8 @@ int CrashyMain(int argc, wchar_t* argv[]) {
       return EXIT_FAILURE;
     }
   } else {
-    fprintf(stderr, "Usage: %s <server_pipe_name>\n", argv[0]);
+    fprintf(stderr, "Usage: %ls <server_pipe_name>\n", argv[0]);
-    fprintf(stderr, "       %s <handler_path> <database_path>\n", argv[0]);
+    fprintf(stderr, "       %ls <handler_path> <database_path>\n", argv[0]);
     return EXIT_FAILURE;
   }
 

snapshot/win/exception_snapshot_win_test.cc

@@ -17,7 +17,6 @@
 #include <string>
 
 #include "base/files/file_path.h"
-#include "base/strings/stringprintf.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
 #include "client/crashpad_client.h"
@@ -39,7 +38,7 @@ namespace {
 // Runs the ExceptionHandlerServer on a background thread.
 class RunServerThread : public Thread {
  public:
-  // Instantiates a thread which will invoke server->Run(delegate, pipe_name);
+  // Instantiates a thread which will invoke server->Run(delegate);
   RunServerThread(ExceptionHandlerServer* server,
                   ExceptionHandlerServer::Delegate* delegate)
       : server_(server), delegate_(delegate) {}
@@ -122,13 +121,12 @@ class CrashingDelegate : public ExceptionHandlerServer::Delegate {
 
 void TestCrashingChild(const base::string16& directory_modification) {
   // Set up the registration server on a background thread.
-  std::string pipe_name = "\\\\.\\pipe\\handler_test_pipe_" +
-                          base::StringPrintf("%08x", GetCurrentProcessId());
   ScopedKernelHANDLE server_ready(CreateEvent(nullptr, false, false, nullptr));
   ScopedKernelHANDLE completed(CreateEvent(nullptr, false, false, nullptr));
   CrashingDelegate delegate(server_ready.get(), completed.get());
 
-  ExceptionHandlerServer exception_handler_server(pipe_name, true);
+  ExceptionHandlerServer exception_handler_server(true);
+  std::wstring pipe_name = exception_handler_server.CreatePipe();
   RunServerThread server_thread(&exception_handler_server, &delegate);
   server_thread.Start();
   ScopedStopServerAndJoinThread scoped_stop_server_and_join_thread(
@@ -144,7 +142,7 @@ void TestCrashingChild(const base::string16& directory_modification) {
           .Append(test_executable.BaseName().RemoveFinalExtension().value() +
                   L"_crashing_child.exe")
           .value();
-  ChildLauncher child(child_test_executable, base::UTF8ToUTF16(pipe_name));
+  ChildLauncher child(child_test_executable, pipe_name);
   child.Start();
 
   // The child tells us (approximately) where it will crash.
@@ -224,13 +222,12 @@ class SimulateDelegate : public ExceptionHandlerServer::Delegate {
 void TestDumpWithoutCrashingChild(
     const base::string16& directory_modification) {
   // Set up the registration server on a background thread.
-  std::string pipe_name = "\\\\.\\pipe\\handler_test_pipe_" +
-                          base::StringPrintf("%08x", GetCurrentProcessId());
   ScopedKernelHANDLE server_ready(CreateEvent(nullptr, false, false, nullptr));
   ScopedKernelHANDLE completed(CreateEvent(nullptr, false, false, nullptr));
   SimulateDelegate delegate(server_ready.get(), completed.get());
 
-  ExceptionHandlerServer exception_handler_server(pipe_name, true);
+  ExceptionHandlerServer exception_handler_server(true);
+  std::wstring pipe_name = exception_handler_server.CreatePipe();
   RunServerThread server_thread(&exception_handler_server, &delegate);
   server_thread.Start();
   ScopedStopServerAndJoinThread scoped_stop_server_and_join_thread(
@@ -246,7 +243,7 @@ void TestDumpWithoutCrashingChild(
           .Append(test_executable.BaseName().RemoveFinalExtension().value() +
                   L"_dump_without_crashing.exe")
           .value();
-  ChildLauncher child(child_test_executable, base::UTF8ToUTF16(pipe_name));
+  ChildLauncher child(child_test_executable, pipe_name);
   child.Start();
 
   // The child tells us (approximately) where it will capture a dump.

util/win/command_line_test.cc

@@ -41,7 +41,7 @@ using ScopedLocalAlloc = base::ScopedGeneric<HLOCAL, LocalAllocTraits>;
 // Calls AppendCommandLineArgument() for every argument in argv, then calls
 // CommandLineToArgvW() to decode the string into a vector again, and compares
 // the input and output.
-void AppendCommandLineArgumentTest(size_t argc, const wchar_t* argv[]) {
+void AppendCommandLineArgumentTest(size_t argc, const wchar_t* const argv[]) {
   std::wstring command_line;
   for (size_t index = 0; index < argc; ++index) {
     AppendCommandLineArgument(argv[index], &command_line);
@@ -69,7 +69,7 @@ TEST(CommandLine, AppendCommandLineArgument) {
   {
     SCOPED_TRACE("simple");
 
-    const wchar_t* kArguments[] = {
+    const wchar_t* const kArguments[] = {
       L"child.exe",
       L"argument 1",
       L"argument 2",
@@ -80,7 +80,7 @@ TEST(CommandLine, AppendCommandLineArgument) {
   {
     SCOPED_TRACE("path with spaces");
 
-    const wchar_t* kArguments[] = {
+    const wchar_t* const kArguments[] = {
       L"child.exe",
       L"argument1",
       L"argument 2",
@@ -92,7 +92,7 @@ TEST(CommandLine, AppendCommandLineArgument) {
   {
     SCOPED_TRACE("argument with embedded quotation marks");
 
-    const wchar_t* kArguments[] = {
+    const wchar_t* const kArguments[] = {
       L"child.exe",
       L"argument1",
       L"she said, \"you had me at hello\"",
@@ -104,7 +104,7 @@ TEST(CommandLine, AppendCommandLineArgument) {
   {
     SCOPED_TRACE("argument with unbalanced quotation marks");
 
-    const wchar_t* kArguments[] = {
+    const wchar_t* const kArguments[] = {
       L"child.exe",
       L"argument1",
       L"argument\"2",
@@ -117,7 +117,7 @@ TEST(CommandLine, AppendCommandLineArgument) {
   {
     SCOPED_TRACE("argument ending with backslash");
 
-    const wchar_t* kArguments[] = {
+    const wchar_t* const kArguments[] = {
       L"child.exe",
       L"\\some\\directory with\\spaces\\",
       L"argument2",
@@ -128,7 +128,7 @@ TEST(CommandLine, AppendCommandLineArgument) {
   {
     SCOPED_TRACE("empty argument");
 
-    const wchar_t* kArguments[] = {
+    const wchar_t* const kArguments[] = {
       L"child.exe",
       L"",
       L"argument2",
@@ -139,7 +139,7 @@ TEST(CommandLine, AppendCommandLineArgument) {
   {
     SCOPED_TRACE("funny nonprintable characters");
 
-    const wchar_t* kArguments[] = {
+    const wchar_t* const kArguments[] = {
       L"child.exe",
       L"argument 1",
       L"argument\t2",

util/win/exception_handler_server.cc

@@ -35,6 +35,29 @@ namespace crashpad {
 
 namespace {
 
+// We create two pipe instances, so that there's one listening while the
+// PipeServiceProc is processing a registration.
+const size_t kPipeInstances = 2;
+
+// Wraps CreateNamedPipe() to create a single named pipe instance.
+//
+// If first_instance is true, the named pipe instance will be created with
+// FILE_FLAG_FIRST_PIPE_INSTANCE. This ensures that the the pipe name is not
+// already in use when created.
+HANDLE CreateNamedPipeInstance(const std::wstring& pipe_name,
+                               bool first_instance) {
+  return CreateNamedPipe(pipe_name.c_str(),
+                         PIPE_ACCESS_DUPLEX |
+                             (first_instance ? FILE_FLAG_FIRST_PIPE_INSTANCE
+                                             : 0),
+                         PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT,
+                         kPipeInstances,
+                         512,
+                         512,
+                         0,
+                         nullptr);
+}
+
 decltype(GetNamedPipeClientProcessId)* GetNamedPipeClientProcessIdFunction() {
   static const auto get_named_pipe_client_process_id =
       GET_FUNCTION(L"kernel32.dll", ::GetNamedPipeClientProcessId);
@@ -234,10 +257,10 @@ class ClientData {
 ExceptionHandlerServer::Delegate::~Delegate() {
 }
 
-ExceptionHandlerServer::ExceptionHandlerServer(const std::string& pipe_name,
+ExceptionHandlerServer::ExceptionHandlerServer(bool persistent)
-                                               bool persistent)
+    : pipe_name_(),
-    : pipe_name_(pipe_name),
       port_(CreateIoCompletionPort(INVALID_HANDLE_VALUE, nullptr, 0, 1)),
+      first_pipe_instance_(),
       clients_lock_(),
       clients_(),
       persistent_(persistent) {
@@ -246,23 +269,59 @@ ExceptionHandlerServer::ExceptionHandlerServer(const std::string& pipe_name,
 ExceptionHandlerServer::~ExceptionHandlerServer() {
 }
 
+void ExceptionHandlerServer::SetPipeName(const std::wstring& pipe_name) {
+  DCHECK(pipe_name_.empty());
+  DCHECK(!pipe_name.empty());
+
+  pipe_name_ = pipe_name;
+}
+
+std::wstring ExceptionHandlerServer::CreatePipe() {
+  DCHECK(!first_pipe_instance_.is_valid());
+
+  int tries = 5;
+  std::string pipe_name_base =
+      base::StringPrintf("\\\\.\\pipe\\crashpad_%d_", GetCurrentProcessId());
+  std::wstring pipe_name;
+  do {
+    pipe_name = base::UTF8ToUTF16(pipe_name_base);
+    for (int index = 0; index < 16; ++index) {
+      pipe_name.append(1, static_cast<wchar_t>(base::RandInt('A', 'Z')));
+    }
+
+    first_pipe_instance_.reset(CreateNamedPipeInstance(pipe_name, true));
+
+    // CreateNamedPipe() is documented as setting the error to
+    // ERROR_ACCESS_DENIED if FILE_FLAG_FIRST_PIPE_INSTANCE is specified and the
+    // pipe name is already in use. However it may set the error to other codes
+    // such as ERROR_PIPE_BUSY (if the pipe already exists and has reached its
+    // maximum instance count) or ERROR_INVALID_PARAMETER (if the pipe already
+    // exists and its attributes differ from those specified to
+    // CreateNamedPipe()). Some of these errors may be ambiguous: for example,
+    // ERROR_INVALID_PARAMETER may also occur if CreateNamedPipe() is called
+    // incorrectly even in the absence of an existing pipe by the same name.
+    //
+    // Rather than chasing down all of the possible errors that might indicate
+    // that a pipe name is already in use, retry up to a few times on any error.
+  } while (!first_pipe_instance_.is_valid() && --tries);
+
+  PCHECK(first_pipe_instance_.is_valid()) << "CreateNamedPipe";
+
+  SetPipeName(pipe_name);
+  return pipe_name;
+}
+
 void ExceptionHandlerServer::Run(Delegate* delegate) {
   uint64_t shutdown_token = base::RandUint64();
-  // We create two pipe instances, so that there's one listening while the
+  ScopedKernelHANDLE thread_handles[kPipeInstances];
-  // PipeServiceProc is processing a registration.
-  ScopedKernelHANDLE thread_handles[2];
-  base::string16 pipe_name_16(base::UTF8ToUTF16(pipe_name_));
   for (int i = 0; i < arraysize(thread_handles); ++i) {
-    HANDLE pipe =
+    HANDLE pipe;
-        CreateNamedPipe(pipe_name_16.c_str(),
+    if (first_pipe_instance_.is_valid()) {
-                        PIPE_ACCESS_DUPLEX,
+      pipe = first_pipe_instance_.release();
-                        PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT,
+    } else {
-                        arraysize(thread_handles),
+      pipe = CreateNamedPipeInstance(pipe_name_, false);
-                        512,
+      PCHECK(pipe != INVALID_HANDLE_VALUE) << "CreateNamedPipe";
-                        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
@@ -313,7 +372,7 @@ void ExceptionHandlerServer::Run(Delegate* delegate) {
     message.type = ClientToServerMessage::kShutdown;
     message.shutdown.token = shutdown_token;
     ServerToClientMessage response;
-    SendToCrashHandlerServer(pipe_name_16,
+    SendToCrashHandlerServer(pipe_name_,
                              reinterpret_cast<ClientToServerMessage&>(message),
                              &response);
   }

util/win/exception_handler_server.h

@@ -20,6 +20,7 @@
 
 #include "base/basictypes.h"
 #include "base/synchronization/lock.h"
+#include "util/file/file_io.h"
 #include "util/win/address_types.h"
 #include "util/win/scoped_handle.h"
 
@@ -62,15 +63,31 @@ class ExceptionHandlerServer {
 
   //! \brief Constructs the exception handling server.
   //!
-  //! \param[in] pipe_name The name of the pipe to listen on. Must be of the
-  //!     form "\\.\pipe\<some_name>".
   //! \param[in] persistent `true` if Run() should not return until Stop() is
   //!     called. If `false`, Run() will return when all clients have exited,
   //!     although Run() will always wait for the first client to connect.
-  ExceptionHandlerServer(const std::string& pipe_name, bool persistent);
+  explicit ExceptionHandlerServer(bool persistent);
 
   ~ExceptionHandlerServer();
 
+  //! \brief Sets the pipe name to listen for client registrations on.
+  //!
+  //! Either this method or CreatePipe(), but not both, must be called before
+  //! Run().
+  //!
+  //! \param[in] pipe_name The name of the pipe to listen on. Must be of the
+  //!     form "\\.\pipe\<some_name>".
+  void SetPipeName(const std::wstring& pipe_name);
+
+  //! \brief Creates a randomized pipe name to listen for client registrations
+  //!     on and returns its name.
+  //!
+  //! Either this method or CreatePipe(), but not both, must be called before
+  //! Run().
+  //!
+  //! \return The pipe name that will be listened on.
+  std::wstring CreatePipe();
+
   //! \brief Runs the exception-handling server.
   //!
   //! \param[in] delegate The interface to which the exceptions are delegated
@@ -89,8 +106,9 @@ class ExceptionHandlerServer {
   static void __stdcall OnNonCrashDumpEvent(void* ctx, BOOLEAN);
   static void __stdcall OnProcessEnd(void* ctx, BOOLEAN);
 
-  std::string pipe_name_;
+  std::wstring pipe_name_;
   ScopedKernelHANDLE port_;
+  ScopedFileHandle first_pipe_instance_;
 
   base::Lock clients_lock_;
   std::set<internal::ClientData*> clients_;

util/win/exception_handler_server_test.cc

@@ -20,7 +20,6 @@
 #include <vector>
 
 #include "base/basictypes.h"
-#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "client/crashpad_client.h"
 #include "gtest/gtest.h"
@@ -80,23 +79,22 @@ class TestDelegate : public ExceptionHandlerServer::Delegate {
 class ExceptionHandlerServerTest : public testing::Test {
  public:
   ExceptionHandlerServerTest()
-      : pipe_name_("\\\\.\\pipe\\exception_handler_server_test_pipe_" +
+      : server_(true),
-                   base::StringPrintf("%08x", GetCurrentProcessId())),
+        pipe_name_(server_.CreatePipe()),
         server_ready_(CreateEvent(nullptr, false, false, nullptr)),
         delegate_(server_ready_.get()),
-        server_(pipe_name_, true),
         server_thread_(&server_, &delegate_) {}
 
   TestDelegate& delegate() { return delegate_; }
   ExceptionHandlerServer& server() { return server_; }
   Thread& server_thread() { return server_thread_; }
-  const std::string& pipe_name() const { return pipe_name_; }
+  const std::wstring& pipe_name() const { return pipe_name_; }
 
  private:
-  std::string pipe_name_;
+  ExceptionHandlerServer server_;
+  std::wstring pipe_name_;
   ScopedKernelHANDLE server_ready_;
   TestDelegate delegate_;
-  ExceptionHandlerServer server_;
   RunServerThread server_thread_;
 
   DISALLOW_COPY_AND_ASSIGN(ExceptionHandlerServerTest);
@@ -135,23 +133,27 @@ TEST_F(ExceptionHandlerServerTest, StopWhileConnected) {
       &server(), &server_thread());
   ASSERT_NO_FATAL_FAILURE(delegate().WaitForStart());
   CrashpadClient client;
-  client.SetHandlerIPCPipe(base::UTF8ToUTF16(pipe_name()));
+  client.SetHandlerIPCPipe(pipe_name());
   // Leaving this scope causes the server to be stopped, while the connection
   // is still open.
 }
 
-std::string ReadString(FileHandle handle) {
+std::wstring ReadWString(FileHandle handle) {
   size_t length = 0;
   EXPECT_TRUE(LoggingReadFile(handle, &length, sizeof(length)));
-  scoped_ptr<char[]> buffer(new char[length]);
+  std::wstring str(length, L'\0');
-  EXPECT_TRUE(LoggingReadFile(handle, &buffer[0], length));
+  if (length > 0) {
-  return std::string(&buffer[0], length);
+    EXPECT_TRUE(LoggingReadFile(handle, &str[0], length * sizeof(str[0])));
+  }
+  return str;
 }
 
-void WriteString(FileHandle handle, const std::string& str) {
+void WriteWString(FileHandle handle, const std::wstring& str) {
   size_t length = str.size();
   EXPECT_TRUE(LoggingWriteFile(handle, &length, sizeof(length)));
-  EXPECT_TRUE(LoggingWriteFile(handle, &str[0], length));
+  if (length > 0) {
+    EXPECT_TRUE(LoggingWriteFile(handle, &str[0], length * sizeof(str[0])));
+  }
 }
 
 class TestClient final : public WinChildProcess {
@@ -162,7 +164,7 @@ class TestClient final : public WinChildProcess {
 
  private:
   int Run() override {
-    std::wstring pipe_name = base::UTF8ToUTF16(ReadString(ReadPipeHandle()));
+    std::wstring pipe_name = ReadWString(ReadPipeHandle());
     CrashpadClient client;
     if (!client.SetHandlerIPCPipe(pipe_name)) {
       ADD_FAILURE();
@@ -172,7 +174,7 @@ class TestClient final : public WinChildProcess {
       ADD_FAILURE();
       return EXIT_FAILURE;
     }
-    WriteString(WritePipeHandle(), "OK");
+    WriteWString(WritePipeHandle(), L"OK");
     return EXIT_SUCCESS;
   }
 
@@ -194,13 +196,13 @@ TEST_F(ExceptionHandlerServerTest, MultipleConnections) {
     ASSERT_NO_FATAL_FAILURE(delegate().WaitForStart());
 
     // Tell all the children where to connect.
-    WriteString(handles_1->write.get(), pipe_name());
+    WriteWString(handles_1->write.get(), pipe_name());
-    WriteString(handles_2->write.get(), pipe_name());
+    WriteWString(handles_2->write.get(), pipe_name());
-    WriteString(handles_3->write.get(), pipe_name());
+    WriteWString(handles_3->write.get(), pipe_name());
 
-    ASSERT_EQ("OK", ReadString(handles_3->read.get()));
+    ASSERT_EQ(L"OK", ReadWString(handles_3->read.get()));
-    ASSERT_EQ("OK", ReadString(handles_2->read.get()));
+    ASSERT_EQ(L"OK", ReadWString(handles_2->read.get()));
-    ASSERT_EQ("OK", ReadString(handles_1->read.get()));
+    ASSERT_EQ(L"OK", ReadWString(handles_1->read.get()));
   }
 }