win: Implement CrashpadClient::StartHandler()

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

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

client/crashpad_client.h

@@ -146,6 +146,8 @@ class CrashpadClient {
  private:
 #if defined(OS_MACOSX)
   base::mac::ScopedMachSendRight exception_port_;
+#elif defined(OS_WIN)
+  std::string ipc_port_;
 #endif
 
   DISALLOW_COPY_AND_ASSIGN(CrashpadClient);

client/crashpad_client_win.cc

@@ -19,10 +19,13 @@
 
 #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"
 #include "base/synchronization/lock.h"
 #include "util/file/file_io.h"
+#include "util/win/command_line.h"
 #include "util/win/critical_section_with_debug_info.h"
 #include "util/win/registration_protocol_win.h"
 #include "util/win/scoped_handle.h"
@@ -102,11 +105,17 @@ LONG WINAPI UnhandledExceptionHandler(EXCEPTION_POINTERS* exception_pointers) {
   return EXCEPTION_CONTINUE_SEARCH;
 }
 
+std::wstring FormatArgumentString(const std::string& name,
+                                  const std::wstring& value) {
+  return std::wstring(L"--") + base::UTF8ToUTF16(name) + L"=" + value;
+}
+
 }  // namespace
 
 namespace crashpad {
 
-CrashpadClient::CrashpadClient() {
+CrashpadClient::CrashpadClient()
+    : ipc_port_() {
 }
 
 CrashpadClient::~CrashpadClient() {
@@ -118,11 +127,81 @@ bool CrashpadClient::StartHandler(
     const std::string& url,
     const std::map<std::string, std::string>& annotations,
     const std::vector<std::string>& arguments) {
-  LOG(FATAL) << "SetHandler should be used on Windows";
+  DCHECK(ipc_port_.empty());
-  return false;
+
+  ipc_port_ =
+      base::StringPrintf("\\\\.\\pipe\\crashpad_%d_", GetCurrentProcessId());
+  for (int index = 0; index < 16; ++index) {
+    ipc_port_.append(1, static_cast<char>(base::RandInt('A', 'Z')));
+  }
+
+  std::wstring command_line;
+  AppendCommandLineArgument(handler.value(), &command_line);
+  for (const std::string& argument : arguments) {
+    AppendCommandLineArgument(base::UTF8ToUTF16(argument), &command_line);
+  }
+  if (!database.value().empty()) {
+    AppendCommandLineArgument(FormatArgumentString("database",
+                                                   database.value()),
+                              &command_line);
+  }
+  if (!url.empty()) {
+    AppendCommandLineArgument(FormatArgumentString("url",
+                                                   base::UTF8ToUTF16(url)),
+                              &command_line);
+  }
+  for (const auto& kv : annotations) {
+    AppendCommandLineArgument(
+        FormatArgumentString("annotation",
+                             base::UTF8ToUTF16(kv.first + '=' + kv.second)),
+        &command_line);
+  }
+  AppendCommandLineArgument(FormatArgumentString("pipe-name",
+                                                 base::UTF8ToUTF16(ipc_port_)),
+                            &command_line);
+
+  STARTUPINFO startup_info = {};
+  startup_info.cb = sizeof(startup_info);
+  startup_info.dwFlags = STARTF_USESTDHANDLES;
+  startup_info.hStdInput = GetStdHandle(STD_INPUT_HANDLE);
+  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(),
+                          &command_line[0],
+                          nullptr,
+                          nullptr,
+                          true,
+                          0,
+                          nullptr,
+                          nullptr,
+                          &startup_info,
+                          &process_info);
+  if (!rv) {
+    PLOG(ERROR) << "CreateProcess";
+    return false;
+  }
+
+  rv = CloseHandle(process_info.hThread);
+  PLOG_IF(WARNING, !rv) << "CloseHandle thread";
+
+  rv = CloseHandle(process_info.hProcess);
+  PLOG_IF(WARNING, !rv) << "CloseHandle process";
+
+  return true;
 }
 
 bool CrashpadClient::SetHandler(const std::string& ipc_port) {
+  DCHECK(ipc_port_.empty());
+  DCHECK(!ipc_port.empty());
+
+  ipc_port_ = ipc_port;
+
+  return true;
+}
+
+bool CrashpadClient::UseHandler() {
+  DCHECK(!ipc_port_.empty());
   DCHECK_EQ(g_signal_exception, INVALID_HANDLE_VALUE);
   DCHECK_EQ(g_signal_non_crash_dump, INVALID_HANDLE_VALUE);
   DCHECK_EQ(g_non_crash_dump_done, INVALID_HANDLE_VALUE);
@@ -154,7 +233,7 @@ bool CrashpadClient::SetHandler(const std::string& ipc_port) {
   ServerToClientMessage response = {0};
 
   if (!SendToCrashHandlerServer(
-          base::UTF8ToUTF16(ipc_port), message, &response)) {
+          base::UTF8ToUTF16(ipc_port_), message, &response)) {
     return false;
   }
 
@@ -168,16 +247,6 @@ bool CrashpadClient::SetHandler(const std::string& ipc_port) {
 
   g_non_crash_dump_lock = new base::Lock();
 
-  return true;
-}
-
-bool CrashpadClient::UseHandler() {
-  if (g_signal_exception == INVALID_HANDLE_VALUE ||
-      g_signal_non_crash_dump == INVALID_HANDLE_VALUE ||
-      g_non_crash_dump_done == INVALID_HANDLE_VALUE) {
-    return false;
-  }
-
   // In theory we could store the previous handler but it is not clear what
   // use we have for it.
   SetUnhandledExceptionFilter(&UnhandledExceptionHandler);
@@ -188,7 +257,7 @@ bool CrashpadClient::UseHandler() {
 void CrashpadClient::DumpWithoutCrash(const CONTEXT& context) {
   if (g_signal_non_crash_dump == INVALID_HANDLE_VALUE ||
       g_non_crash_dump_done == INVALID_HANDLE_VALUE) {
-    LOG(ERROR) << "haven't called SetHandler()";
+    LOG(ERROR) << "haven't called UseHandler()";
     return;
   }
 

handler/crashpad_handler.ad

@@ -33,14 +33,23 @@ collection server. Uploads are disabled by default, and can only be enabled by a
 Crashpad client using the Crashpad client library, typically in response to a
 user requesting this behavior.
 
-This server is normally started by its initial client, and it performs a
+On OS X, this server is normally started by its initial client, and it performs
-handshake with this client via a pipe established by the client that is
+a handshake with this client via a pipe established by the client that is
 inherited by the server, referenced by the *--handshake-fd* argument. During the
 handshake, the server furnishes the client with a send right that the client may
 use as an exception port. The server retains the corresponding receive right,
 which it monitors for exception messages. When the receive right loses all
 senders, the server exits after allowing any upload in progress to complete.
 
+On Windows, clients register with this server by communicating with it via the
+named pipe identified by the *--pipe-name* argument. During registration, a
+client provides the server with an OS event object that it will signal should it
+crash. The server obtains the client’s process handle and waits on the crash
+event object for a crash, as well as the client’s process handle for the client
+to exit cleanly without crashing. When the server loses all clients and
+*--persistent* is not specified, 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. Arbitrary
 programs may be run with a Crashpad handler by using
@@ -77,6 +86,12 @@ of 'PATH' exists.
 Perform the handshake with the initial client on the file descriptor at 'FD'.
 This option is required. This option is only valid on Mac OS X.
 
+*--persistent*::
+Continue running after the last client exits. If this option is not specified,
+this server will exit as soon as it has no clients, although on startup, it
+always waits for at least one client to connect. This option is only valid on
+Windows.
+
 *--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

handler/main.cc

@@ -60,6 +60,7 @@ 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"
 "      --pipe-name=PIPE        communicate with the client over PIPE\n"
 #endif  // OS_MACOSX
 "      --url=URL               send crash reports to this Breakpad server URL,\n"
@@ -84,6 +85,7 @@ int HandlerMain(int argc, char* argv[]) {
     kOptionHandshakeFD,
     kOptionResetOwnCrashExceptionPortToSystemDefault,
 #elif defined(OS_WIN)
+    kOptionPersistent,
     kOptionPipeName,
 #endif  // OS_MACOSX
     kOptionURL,
@@ -101,8 +103,9 @@ int HandlerMain(int argc, char* argv[]) {
     int handshake_fd;
     bool reset_own_crash_exception_port_to_system_default;
 #elif defined(OS_WIN)
+    bool persistent;
     std::string pipe_name;
-#endif
+#endif  // OS_MACOSX
   } options = {};
 #if defined(OS_MACOSX)
   options.handshake_fd = -1;
@@ -119,8 +122,9 @@ int HandlerMain(int argc, char* argv[]) {
        nullptr,
        kOptionResetOwnCrashExceptionPortToSystemDefault},
 #elif defined(OS_WIN)
+      {"persistent", no_argument, nullptr, kOptionPersistent},
       {"pipe-name", required_argument, nullptr, kOptionPipeName},
-#endif
+#endif  // OS_MACOSX
       {"url", required_argument, nullptr, kOptionURL},
       {"help", no_argument, nullptr, kOptionHelp},
       {"version", no_argument, nullptr, kOptionVersion},
@@ -163,6 +167,10 @@ int HandlerMain(int argc, char* argv[]) {
         break;
       }
 #elif defined(OS_WIN)
+      case kOptionPersistent: {
+        options.persistent = true;
+        break;
+      }
       case kOptionPipeName: {
         options.pipe_name = optarg;
         break;
@@ -228,7 +236,8 @@ int HandlerMain(int argc, char* argv[]) {
 
   ExceptionHandlerServer exception_handler_server(receive_right.Pass());
 #elif defined(OS_WIN)
-  ExceptionHandlerServer exception_handler_server(options.pipe_name);
+  ExceptionHandlerServer exception_handler_server(options.pipe_name,
+                                                  options.persistent);
 #endif  // OS_MACOSX
 
   scoped_ptr<CrashReportDatabase> database(CrashReportDatabase::Initialize(

handler/win/crashy_test_program.cc

@@ -12,16 +12,23 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <stdlib.h>
 #include <windows.h>
 #include <winternl.h>
 
+#include <string>
+#include <map>
+#include <vector>
+
 // ntstatus.h conflicts with windows.h so define this locally.
 #ifndef STATUS_NO_SUCH_FILE
 #define STATUS_NO_SUCH_FILE static_cast<NTSTATUS>(0xC000000F)
 #endif
 
 #include "base/basictypes.h"
+#include "base/files/file_path.h"
 #include "base/logging.h"
+#include "base/strings/utf_string_conversions.h"
 #include "client/crashpad_client.h"
 #include "tools/tool_support.h"
 #include "util/win/critical_section_with_debug_info.h"
@@ -88,19 +95,31 @@ void SomeCrashyFunction() {
 }
 
 int CrashyMain(int argc, char* argv[]) {
-  if (argc != 2) {
+  CrashpadClient client;
+
+  if (argc == 2) {
+    if (!client.SetHandler(argv[1])) {
+      LOG(ERROR) << "SetHandler";
+      return EXIT_FAILURE;
+    }
+  } else if (argc == 3) {
+    if (!client.StartHandler(base::FilePath(base::UTF8ToUTF16(argv[1])),
+                             base::FilePath(base::UTF8ToUTF16(argv[2])),
+                             std::string(),
+                             std::map<std::string, std::string>(),
+                             std::vector<std::string>())) {
+      LOG(ERROR) << "StartHandler";
+      return EXIT_FAILURE;
+    }
+  } else {
     fprintf(stderr, "Usage: %s <server_pipe_name>\n", argv[0]);
-    return 1;
+    fprintf(stderr, "       %s <handler_path> <database_path>\n", argv[0]);
+    return EXIT_FAILURE;
   }
 
-  CrashpadClient client;
-  if (!client.SetHandler(argv[1])) {
-    LOG(ERROR) << "SetHandler";
-    return 1;
-  }
   if (!client.UseHandler()) {
     LOG(ERROR) << "UseHandler";
-    return 1;
+    return EXIT_FAILURE;
   }
 
   AllocateMemoryOfVariousProtections();
@@ -112,7 +131,7 @@ int CrashyMain(int argc, char* argv[]) {
 
   SomeCrashyFunction();
 
-  return 0;
+  return EXIT_SUCCESS;
 }
 
 }  // namespace

snapshot/win/end_to_end_test.py

@@ -78,9 +78,11 @@ def GetCdbPath():
 
 
 def GetDumpFromProgram(out_dir, pipe_name, executable_name):
-  """Initialize a crash database, run crashpad_handler, run |executable_name|
+  """Initialize a crash database, and run |executable_name| connecting to a
-  connecting to the crash_handler. Returns the minidump generated by
+  crash handler. If pipe_name is set, crashpad_handler will be started first. If
-  crash_handler for further testing.
+  pipe_name is empty, the executable is responsible for starting
+  crashpad_handler. Returns the minidump generated by crashpad_handler for
+  further testing.
   """
   test_database = MakeTempDir()
   handler = None
@@ -92,13 +94,18 @@ def GetDumpFromProgram(out_dir, pipe_name, executable_name):
       print 'could not initialize report database'
       return None
 
-    handler = subprocess.Popen([
+    if pipe_name is not None:
-        os.path.join(out_dir, 'crashpad_handler.exe'),
+      handler = subprocess.Popen([
-        '--pipe-name=' + pipe_name,
+          os.path.join(out_dir, 'crashpad_handler.exe'),
-        '--database=' + test_database
+          '--pipe-name=' + pipe_name,
-    ])
+          '--database=' + test_database
+      ])
 
-    subprocess.call([os.path.join(out_dir, executable_name), pipe_name])
+      subprocess.call([os.path.join(out_dir, executable_name), pipe_name])
+    else:
+      subprocess.call([os.path.join(out_dir, executable_name),
+                       os.path.join(out_dir, 'crashpad_handler.exe'),
+                       test_database])
 
     out = subprocess.check_output([
         os.path.join(out_dir, 'crashpad_database_util.exe'),
@@ -160,7 +167,12 @@ class CdbRun(object):
       sys.exit(1)
 
 
-def RunTests(cdb_path, dump_path, destroyed_dump_path, z7_dump_path, pipe_name):
+def RunTests(cdb_path,
+             dump_path,
+             start_handler_dump_path,
+             destroyed_dump_path,
+             z7_dump_path,
+             pipe_name):
   """Runs various tests in sequence. Runs a new cdb instance on the dump for
   each block of tests to reduce the chances that output from one command is
   confused for output from another.
@@ -172,6 +184,13 @@ def RunTests(cdb_path, dump_path, destroyed_dump_path, z7_dump_path, pipe_name):
       'crashy_program!crashpad::`anonymous namespace\'::SomeCrashyFunction',
       'exception at correct location')
 
+  out = CdbRun(cdb_path, start_handler_dump_path, '.ecxr')
+  out.Check('This dump file has an exception of interest stored in it',
+            'captured exception (using StartHandler())')
+  out.Check(
+      'crashy_program!crashpad::`anonymous namespace\'::SomeCrashyFunction',
+      'exception at correct location (using StartHandler())')
+
   out = CdbRun(cdb_path, dump_path, '!peb')
   out.Check(r'PEB at', 'found the PEB')
   out.Check(r'Ldr\.InMemoryOrderModuleList:.*\d+ \. \d+', 'PEB_LDR_DATA saved')
@@ -255,6 +274,10 @@ def main(args):
     if not crashy_dump_path:
       return 1
 
+    start_handler_dump_path = GetDumpFromCrashyProgram(args[0], None)
+    if not start_handler_dump_path:
+      return 1
+
     destroyed_dump_path = GetDumpFromSelfDestroyingProgram(args[0], pipe_name)
     if not destroyed_dump_path:
       return 1
@@ -265,7 +288,11 @@ def main(args):
       if not z7_dump_path:
         return 1
 
-    RunTests(cdb_path, crashy_dump_path, destroyed_dump_path, z7_dump_path,
+    RunTests(cdb_path,
+             crashy_dump_path,
+             start_handler_dump_path,
+             destroyed_dump_path,
+             z7_dump_path,
              pipe_name)
 
     return 0

snapshot/win/exception_snapshot_win_test.cc

@@ -128,7 +128,7 @@ void TestCrashingChild(const base::string16& directory_modification) {
   ScopedKernelHANDLE completed(CreateEvent(nullptr, false, false, nullptr));
   CrashingDelegate delegate(server_ready.get(), completed.get());
 
-  ExceptionHandlerServer exception_handler_server(pipe_name);
+  ExceptionHandlerServer exception_handler_server(pipe_name, true);
   RunServerThread server_thread(&exception_handler_server, &delegate);
   server_thread.Start();
   ScopedStopServerAndJoinThread scoped_stop_server_and_join_thread(
@@ -230,7 +230,7 @@ void TestDumpWithoutCrashingChild(
   ScopedKernelHANDLE completed(CreateEvent(nullptr, false, false, nullptr));
   SimulateDelegate delegate(server_ready.get(), completed.get());
 
-  ExceptionHandlerServer exception_handler_server(pipe_name);
+  ExceptionHandlerServer exception_handler_server(pipe_name, true);
   RunServerThread server_thread(&exception_handler_server, &delegate);
   server_thread.Start();
   ScopedStopServerAndJoinThread scoped_stop_server_and_join_thread(

test/multiprocess_exec_win.cc

@@ -17,7 +17,7 @@
 #include "base/logging.h"
 #include "base/strings/utf_string_conversions.h"
 #include "gtest/gtest.h"
-#include "test/win/child_launcher.h"
+#include "util/win/command_line.h"
 
 namespace crashpad {
 namespace test {
@@ -119,7 +119,6 @@ void MultiprocessExec::PreFork() {
   command_line_.clear();
   AppendCommandLineArgument(base::UTF8ToUTF16(command_), &command_line_);
   for (size_t i = 0; i < arguments_.size(); ++i) {
-    command_line_ += L" ";
     AppendCommandLineArgument(base::UTF8ToUTF16(arguments_[i]), &command_line_);
   }
 

test/win/child_launcher.cc

@@ -15,6 +15,7 @@
 #include "test/win/child_launcher.h"
 
 #include "gtest/gtest.h"
+#include "util/win/command_line.h"
 
 namespace crashpad {
 namespace test {
@@ -96,39 +97,5 @@ DWORD ChildLauncher::WaitForExit() {
   return exit_code;
 }
 
-// Ref: http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx
-void AppendCommandLineArgument(const std::wstring& argument,
-                               std::wstring* command_line) {
-  // Don't bother quoting if unnecessary.
-  if (!argument.empty() &&
-      argument.find_first_of(L" \t\n\v\"") == std::wstring::npos) {
-    command_line->append(argument);
-  } else {
-    command_line->push_back(L'"');
-    for (std::wstring::const_iterator i = argument.begin();; ++i) {
-      size_t backslash_count = 0;
-      while (i != argument.end() && *i == L'\\') {
-        ++i;
-        ++backslash_count;
-      }
-      if (i == argument.end()) {
-        // Escape all backslashes, but let the terminating double quotation mark
-        // we add below be interpreted as a metacharacter.
-        command_line->append(backslash_count * 2, L'\\');
-        break;
-      } else if (*i == L'"') {
-        // Escape all backslashes and the following double quotation mark.
-        command_line->append(backslash_count * 2 + 1, L'\\');
-        command_line->push_back(*i);
-      } else {
-        // Backslashes aren't special here.
-        command_line->append(backslash_count, L'\\');
-        command_line->push_back(*i);
-      }
-    }
-    command_line->push_back(L'"');
-  }
-}
-
 }  // namespace test
 }  // namespace crashpad

test/win/child_launcher.h

@@ -67,14 +67,6 @@ class ChildLauncher {
   ScopedFileHANDLE stdin_write_handle_;
 };
 
-//! \brief Utility function for building escaped command lines.
-//!
-//! \param[in] argument Appended to \a command_line surrounded by properly
-//!     escaped quotation marks, if necessary.
-//! \param[inout] command_line The command line being constructed.
-void AppendCommandLineArgument(const std::wstring& argument,
-                               std::wstring* command_line);
-
 }  // namespace test
 }  // namespace crashpad
 

util/util.gyp

@@ -152,6 +152,8 @@
         'win/capture_context.asm',
         'win/capture_context.h',
         'win/checked_win_address_range.h',
+        'win/command_line.cc',
+        'win/command_line.h',
         'win/critical_section_with_debug_info.cc',
         'win/critical_section_with_debug_info.h',
         'win/exception_handler_server.cc',

util/util_test.gyp

@@ -80,6 +80,7 @@
         'thread/thread_log_messages_test.cc',
         'thread/thread_test.cc',
         'win/capture_context_test.cc',
+        'win/command_line_test.cc',
         'win/critical_section_with_debug_info_test.cc',
         'win/exception_handler_server_test.cc',
         'win/get_function_test.cc',

util/win/command_line.cc

@@ -0,0 +1,58 @@
+// Copyright 2015 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "util/win/command_line.h"
+
+namespace crashpad {
+
+// Ref:
+// http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx
+void AppendCommandLineArgument(const std::wstring& argument,
+                               std::wstring* command_line) {
+  if (!command_line->empty()) {
+    command_line->push_back(L' ');
+  }
+
+  // Don’t bother quoting if unnecessary.
+  if (!argument.empty() &&
+      argument.find_first_of(L" \t\n\v\"") == std::wstring::npos) {
+    command_line->append(argument);
+  } else {
+    command_line->push_back(L'"');
+    for (std::wstring::const_iterator i = argument.begin();; ++i) {
+      size_t backslash_count = 0;
+      while (i != argument.end() && *i == L'\\') {
+        ++i;
+        ++backslash_count;
+      }
+      if (i == argument.end()) {
+        // Escape all backslashes, but let the terminating double quotation mark
+        // we add below be interpreted as a metacharacter.
+        command_line->append(backslash_count * 2, L'\\');
+        break;
+      } else if (*i == L'"') {
+        // Escape all backslashes and the following double quotation mark.
+        command_line->append(backslash_count * 2 + 1, L'\\');
+        command_line->push_back(*i);
+      } else {
+        // Backslashes aren’t special here.
+        command_line->append(backslash_count, L'\\');
+        command_line->push_back(*i);
+      }
+    }
+    command_line->push_back(L'"');
+  }
+}
+
+}  // namespace crashpad

util/win/command_line.h

@@ -0,0 +1,38 @@
+// Copyright 2015 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef CRASHPAD_UTIL_WIN_COMMAND_LINE_H_
+#define CRASHPAD_UTIL_WIN_COMMAND_LINE_H_
+
+#include <string>
+
+namespace crashpad {
+
+//! \brief Utility function for building escaped command lines.
+//!
+//! This builds a command line so that individual arguments can be reliably
+//! decoded by `CommandLineToArgvW()`.
+//!
+//! \a argument is appended to \a command_line. If necessary, it will be placed
+//! in quotation marks and escaped properly. If \a command_line is initially
+//! non-empty, a space will precede \a argument.
+//!
+//! \param[in] argument The argument to append to \a command_line.
+//! \param[inout] command_line The command line being constructed.
+void AppendCommandLineArgument(const std::wstring& argument,
+                               std::wstring* command_line);
+
+}  // namespace crashpad
+
+#endif  // CRASHPAD_UTIL_WIN_COMMAND_LINE_H_

util/win/command_line_test.cc

@@ -0,0 +1,177 @@
+// Copyright 2015 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "util/win/command_line.h"
+
+#include <windows.h>
+#include <shellapi.h>
+
+#include "base/basictypes.h"
+#include "base/logging.h"
+#include "base/scoped_generic.h"
+#include "gtest/gtest.h"
+#include "test/errors.h"
+
+namespace crashpad {
+namespace test {
+namespace {
+
+struct LocalAllocTraits {
+  static HLOCAL InvalidValue() {
+    return nullptr;
+  }
+
+  static void Free(HLOCAL memory) {
+    PLOG_IF(ERROR, LocalFree(memory) != nullptr) << "LocalFree";
+  }
+};
+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[]) {
+  std::wstring command_line;
+  for (size_t index = 0; index < argc; ++index) {
+    AppendCommandLineArgument(argv[index], &command_line);
+  }
+
+  int test_argc;
+  wchar_t** test_argv = CommandLineToArgvW(command_line.c_str(), &test_argc);
+
+  ASSERT_TRUE(test_argv) << ErrorMessage("CommandLineToArgvW");
+  ScopedLocalAlloc test_argv_owner(test_argv);
+  ASSERT_EQ(argc, test_argc);
+
+  for (size_t index = 0; index < argc; ++index) {
+    EXPECT_STREQ(argv[index], test_argv[index]) << "index " << index;
+  }
+  EXPECT_FALSE(test_argv[argc]);
+}
+
+TEST(CommandLine, AppendCommandLineArgument) {
+  // Most of these test cases come from
+  // http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx,
+  // which was also a reference for the implementation of
+  // AppendCommandLineArgument().
+
+  {
+    SCOPED_TRACE("simple");
+
+    const wchar_t* kArguments[] = {
+      L"child.exe",
+      L"argument 1",
+      L"argument 2",
+    };
+    AppendCommandLineArgumentTest(arraysize(kArguments), kArguments);
+  }
+
+  {
+    SCOPED_TRACE("path with spaces");
+
+    const wchar_t* kArguments[] = {
+      L"child.exe",
+      L"argument1",
+      L"argument 2",
+      L"\\some\\path with\\spaces",
+    };
+    AppendCommandLineArgumentTest(arraysize(kArguments), kArguments);
+  }
+
+  {
+    SCOPED_TRACE("argument with embedded quotation marks");
+
+    const wchar_t* kArguments[] = {
+      L"child.exe",
+      L"argument1",
+      L"she said, \"you had me at hello\"",
+      L"\\some\\path with\\spaces",
+    };
+    AppendCommandLineArgumentTest(arraysize(kArguments), kArguments);
+  }
+
+  {
+    SCOPED_TRACE("argument with unbalanced quotation marks");
+
+    const wchar_t* kArguments[] = {
+      L"child.exe",
+      L"argument1",
+      L"argument\"2",
+      L"argument3",
+      L"argument4",
+    };
+    AppendCommandLineArgumentTest(arraysize(kArguments), kArguments);
+  }
+
+  {
+    SCOPED_TRACE("argument ending with backslash");
+
+    const wchar_t* kArguments[] = {
+      L"child.exe",
+      L"\\some\\directory with\\spaces\\",
+      L"argument2",
+    };
+    AppendCommandLineArgumentTest(arraysize(kArguments), kArguments);
+  }
+
+  {
+    SCOPED_TRACE("empty argument");
+
+    const wchar_t* kArguments[] = {
+      L"child.exe",
+      L"",
+      L"argument2",
+    };
+    AppendCommandLineArgumentTest(arraysize(kArguments), kArguments);
+  }
+
+  {
+    SCOPED_TRACE("funny nonprintable characters");
+
+    const wchar_t* kArguments[] = {
+      L"child.exe",
+      L"argument 1",
+      L"argument\t2",
+      L"argument\n3",
+      L"argument\v4",
+      L"argument\"5",
+      L" ",
+      L"\t",
+      L"\n",
+      L"\v",
+      L"\"",
+      L" x",
+      L"\tx",
+      L"\nx",
+      L"\vx",
+      L"\"x",
+      L"x ",
+      L"x\t",
+      L"x\n",
+      L"x\v",
+      L"x\"",
+      L" ",
+      L"\t\t",
+      L"\n\n",
+      L"\v\v",
+      L"\"\"",
+      L" \t\n\v\"",
+    };
+    AppendCommandLineArgumentTest(arraysize(kArguments), kArguments);
+  }
+}
+
+}  // namespace
+}  // namespace test
+}  // namespace crashpad

util/win/exception_handler_server.cc

@@ -234,11 +234,13 @@ class ClientData {
 ExceptionHandlerServer::Delegate::~Delegate() {
 }
 
-ExceptionHandlerServer::ExceptionHandlerServer(const std::string& pipe_name)
+ExceptionHandlerServer::ExceptionHandlerServer(const std::string& pipe_name,
+                                               bool persistent)
     : pipe_name_(pipe_name),
       port_(CreateIoCompletionPort(INVALID_HANDLE_VALUE, nullptr, 0, 1)),
       clients_lock_(),
-      clients_() {
+      clients_(),
+      persistent_(persistent) {
 }
 
 ExceptionHandlerServer::~ExceptionHandlerServer() {
@@ -297,11 +299,11 @@ void ExceptionHandlerServer::Run(Delegate* delegate) {
     // outstanding threadpool waits are complete. This is important because the
     // process handle can be signalled *before* the dump request is signalled.
     internal::ClientData* client = reinterpret_cast<internal::ClientData*>(key);
-    {
+    base::AutoLock lock(clients_lock_);
-      base::AutoLock lock(clients_lock_);
+    clients_.erase(client);
-      clients_.erase(client);
-    }
     delete client;
+    if (!persistent_ && clients_.empty())
+      break;
   }
 
   // Signal to the named pipe instances that they should terminate.

util/win/exception_handler_server.h

@@ -64,7 +64,10 @@ class ExceptionHandlerServer {
   //!
   //! \param[in] pipe_name The name of the pipe to listen on. Must be of the
   //!     form "\\.\pipe\<some_name>".
-  explicit ExceptionHandlerServer(const std::string& pipe_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);
 
   ~ExceptionHandlerServer();
 
@@ -92,6 +95,8 @@ class ExceptionHandlerServer {
   base::Lock clients_lock_;
   std::set<internal::ClientData*> clients_;
 
+  bool persistent_;
+
   DISALLOW_COPY_AND_ASSIGN(ExceptionHandlerServer);
 };
 

util/win/exception_handler_server_test.cc

@@ -83,7 +83,7 @@ class ExceptionHandlerServerTest : public testing::Test {
                    base::StringPrintf("%08x", GetCurrentProcessId())),
         server_ready_(CreateEvent(nullptr, false, false, nullptr)),
         delegate_(server_ready_.get()),
-        server_(pipe_name_),
+        server_(pipe_name_, true),
         server_thread_(&server_, &delegate_) {}
 
   TestDelegate& delegate() { return delegate_; }

util/win/process_info_test.cc

@@ -31,6 +31,7 @@
 #include "test/win/child_launcher.h"
 #include "util/file/file_io.h"
 #include "util/misc/uuid.h"
+#include "util/win/command_line.h"
 #include "util/win/get_function.h"
 #include "util/win/scoped_handle.h"