diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc index 01df466f..a2506cb7 100644 --- a/client/crashpad_client_win.cc +++ b/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 = - base::StringPrintf("\\\\.\\pipe\\crashpad_%d_", GetCurrentProcessId()); - for (int index = 0; index < 16; ++index) { - ipc_pipe.append(1, static_cast(base::RandInt('A', 'Z'))); + HANDLE pipe_read; + HANDLE pipe_write; + SECURITY_ATTRIBUTES security_attributes = {}; + 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(), - &command_line[0], - nullptr, - nullptr, - true, - 0, - nullptr, - nullptr, - &startup_info, - &process_info); + 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; @@ -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; } diff --git a/handler/crashpad_handler.ad b/handler/crashpad_handler.ad index f359af76..e7fd66ba 100644 --- a/handler/crashpad_handler.ad +++ b/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 -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. +named pipe identified by the *--pipe-name* argument. Alternatively, the server +can create a new pipe with a random name and inform a client of this name via +the *--handshake-handle* mechanism; clients may then register by communicating +with it via that named pipe. 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 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*:: -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. +*--handshake-handle*='HANDLE':: +Perform the handshake with the initial client on the HANDLE at 'HANDLE'. Either +this option or *--pipe-name*, but not both, is required. This option is only +valid on 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\+. This option is required. This option is only -valid on Windows. +the form +\\.\pipe\+. Either this option or *--handshake-handle*, but +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 diff --git a/handler/main.cc b/handler/main.cc index 879122d5..7b1f7304 100644 --- a/handler/main.cc +++ b/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: { - options.persistent = true; + case kOptionHandshakeHandle: { + // 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( + static_cast(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()) { - ToolSupport::UsageHint(me, "--pipe-name is required"); + if (options.handshake_handle == INVALID_HANDLE_VALUE && + 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, - options.persistent); + ExceptionHandlerServer exception_handler_server(!options.pipe_name.empty()); + + 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(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 database(CrashReportDatabase::Initialize( diff --git a/handler/win/crashy_test_program.cc b/handler/win/crashy_test_program.cc index 921638d9..847093a3 100644 --- a/handler/win/crashy_test_program.cc +++ b/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 \n", argv[0]); - fprintf(stderr, " %s \n", argv[0]); + fprintf(stderr, "Usage: %ls \n", argv[0]); + fprintf(stderr, " %ls \n", argv[0]); return EXIT_FAILURE; } diff --git a/snapshot/win/exception_snapshot_win_test.cc b/snapshot/win/exception_snapshot_win_test.cc index 8e2bc61b..5c3b0609 100644 --- a/snapshot/win/exception_snapshot_win_test.cc +++ b/snapshot/win/exception_snapshot_win_test.cc @@ -17,7 +17,6 @@ #include #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. diff --git a/util/win/command_line_test.cc b/util/win/command_line_test.cc index 5dc9e901..f3955697 100644 --- a/util/win/command_line_test.cc +++ b/util/win/command_line_test.cc @@ -41,7 +41,7 @@ using ScopedLocalAlloc = base::ScopedGeneric; // 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", diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc index 50daa39a..2035c578 100644 --- a/util/win/exception_handler_server.cc +++ b/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, - bool persistent) - : pipe_name_(pipe_name), +ExceptionHandlerServer::ExceptionHandlerServer(bool persistent) + : 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(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 - // PipeServiceProc is processing a registration. - ScopedKernelHANDLE thread_handles[2]; - base::string16 pipe_name_16(base::UTF8ToUTF16(pipe_name_)); + ScopedKernelHANDLE thread_handles[kPipeInstances]; for (int i = 0; i < arraysize(thread_handles); ++i) { - HANDLE pipe = - CreateNamedPipe(pipe_name_16.c_str(), - PIPE_ACCESS_DUPLEX, - PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT, - arraysize(thread_handles), - 512, - 512, - 0, - nullptr); - PCHECK(pipe != INVALID_HANDLE_VALUE) << "CreateNamedPipe"; + HANDLE pipe; + if (first_pipe_instance_.is_valid()) { + pipe = first_pipe_instance_.release(); + } else { + pipe = CreateNamedPipeInstance(pipe_name_, false); + 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(message), &response); } diff --git a/util/win/exception_handler_server.h b/util/win/exception_handler_server.h index d4a81f16..46141414 100644 --- a/util/win/exception_handler_server.h +++ b/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\". //! \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\". + 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 clients_; diff --git a/util/win/exception_handler_server_test.cc b/util/win/exception_handler_server_test.cc index 8cfbb45f..ee31e26d 100644 --- a/util/win/exception_handler_server_test.cc +++ b/util/win/exception_handler_server_test.cc @@ -20,7 +20,6 @@ #include #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_" + - base::StringPrintf("%08x", GetCurrentProcessId())), + : server_(true), + 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 buffer(new char[length]); - EXPECT_TRUE(LoggingReadFile(handle, &buffer[0], length)); - return std::string(&buffer[0], length); + std::wstring str(length, L'\0'); + if (length > 0) { + 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()); - WriteString(handles_2->write.get(), pipe_name()); - WriteString(handles_3->write.get(), pipe_name()); + WriteWString(handles_1->write.get(), pipe_name()); + WriteWString(handles_2->write.get(), pipe_name()); + WriteWString(handles_3->write.get(), pipe_name()); - ASSERT_EQ("OK", ReadString(handles_3->read.get())); - ASSERT_EQ("OK", ReadString(handles_2->read.get())); - ASSERT_EQ("OK", ReadString(handles_1->read.get())); + ASSERT_EQ(L"OK", ReadWString(handles_3->read.get())); + ASSERT_EQ(L"OK", ReadWString(handles_2->read.get())); + ASSERT_EQ(L"OK", ReadWString(handles_1->read.get())); } }