From 9d03d54d0ba1aa19d90ee226bb6fec9f901d2725 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Thu, 29 Oct 2015 15:12:23 -0400 Subject: [PATCH] win: Construct ExceptionHandlerServer() with its pipe argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows better code sharing in crashpad_handler’s main(). It doesn’t look like much of an improvement now, but a separate change will cause the Mac ExceptionHandlerServer() to be constructed with an argument. It will be beneficial for Mac and Windows to be able to share the Run() call. R=scottmg@chromium.org Review URL: https://codereview.chromium.org/1402333004 . --- handler/main.cc | 8 +++----- snapshot/win/exception_snapshot_win_test.cc | 18 +++++++----------- util/win/exception_handler_server.cc | 12 ++++++------ util/win/exception_handler_server.h | 11 +++++++---- util/win/exception_handler_server_test.cc | 14 ++++++-------- 5 files changed, 29 insertions(+), 34 deletions(-) diff --git a/handler/main.cc b/handler/main.cc index 860951dc..64a49d56 100644 --- a/handler/main.cc +++ b/handler/main.cc @@ -216,9 +216,9 @@ int HandlerMain(int argc, char* argv[]) { } #endif +#if defined(OS_MACOSX) ExceptionHandlerServer exception_handler_server; -#if defined(OS_MACOSX) CloseStdinAndStdout(); if (!ChildPortHandshake::RunClientForFD( base::ScopedFD(options.handshake_fd), @@ -226,6 +226,8 @@ int HandlerMain(int argc, char* argv[]) { MACH_MSG_TYPE_MAKE_SEND)) { return EXIT_FAILURE; } +#elif defined(OS_WIN) + ExceptionHandlerServer exception_handler_server(options.pipe_name); #endif // OS_MACOSX scoped_ptr database(CrashReportDatabase::Initialize( @@ -241,11 +243,7 @@ int HandlerMain(int argc, char* argv[]) { CrashReportExceptionHandler exception_handler( database.get(), &upload_thread, &options.annotations); -#if defined(OS_MACOSX) exception_handler_server.Run(&exception_handler); -#elif defined(OS_WIN) - exception_handler_server.Run(&exception_handler, options.pipe_name); -#endif // OS_MACOSX upload_thread.Stop(); diff --git a/snapshot/win/exception_snapshot_win_test.cc b/snapshot/win/exception_snapshot_win_test.cc index 7c3b3691..fe626a38 100644 --- a/snapshot/win/exception_snapshot_win_test.cc +++ b/snapshot/win/exception_snapshot_win_test.cc @@ -41,18 +41,16 @@ class RunServerThread : public Thread { public: // Instantiates a thread which will invoke server->Run(delegate, pipe_name); RunServerThread(ExceptionHandlerServer* server, - ExceptionHandlerServer::Delegate* delegate, - const std::string& pipe_name) - : server_(server), delegate_(delegate), pipe_name_(pipe_name) {} + ExceptionHandlerServer::Delegate* delegate) + : server_(server), delegate_(delegate) {} ~RunServerThread() override {} private: // Thread: - void ThreadMain() override { server_->Run(delegate_, pipe_name_); } + void ThreadMain() override { server_->Run(delegate_); } ExceptionHandlerServer* server_; ExceptionHandlerServer::Delegate* delegate_; - std::string pipe_name_; DISALLOW_COPY_AND_ASSIGN(RunServerThread); }; @@ -130,9 +128,8 @@ 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; - RunServerThread server_thread( - &exception_handler_server, &delegate, pipe_name); + ExceptionHandlerServer exception_handler_server(pipe_name); + RunServerThread server_thread(&exception_handler_server, &delegate); server_thread.Start(); ScopedStopServerAndJoinThread scoped_stop_server_and_join_thread( &exception_handler_server, &server_thread); @@ -233,9 +230,8 @@ void TestDumpWithoutCrashingChild( ScopedKernelHANDLE completed(CreateEvent(nullptr, false, false, nullptr)); SimulateDelegate delegate(server_ready.get(), completed.get()); - ExceptionHandlerServer exception_handler_server; - RunServerThread server_thread( - &exception_handler_server, &delegate, pipe_name); + ExceptionHandlerServer exception_handler_server(pipe_name); + RunServerThread server_thread(&exception_handler_server, &delegate); server_thread.Start(); ScopedStopServerAndJoinThread scoped_stop_server_and_join_thread( &exception_handler_server, &server_thread); diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc index aba3ffa0..234d3149 100644 --- a/util/win/exception_handler_server.cc +++ b/util/win/exception_handler_server.cc @@ -234,8 +234,9 @@ class ClientData { ExceptionHandlerServer::Delegate::~Delegate() { } -ExceptionHandlerServer::ExceptionHandlerServer() - : port_(CreateIoCompletionPort(INVALID_HANDLE_VALUE, nullptr, 0, 1)), +ExceptionHandlerServer::ExceptionHandlerServer(const std::string& pipe_name) + : pipe_name_(pipe_name), + port_(CreateIoCompletionPort(INVALID_HANDLE_VALUE, nullptr, 0, 1)), clients_lock_(), clients_() { } @@ -243,13 +244,12 @@ ExceptionHandlerServer::ExceptionHandlerServer() ExceptionHandlerServer::~ExceptionHandlerServer() { } -void ExceptionHandlerServer::Run(Delegate* delegate, - const std::string& 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)); + base::string16 pipe_name_16(base::UTF8ToUTF16(pipe_name_)); for (int i = 0; i < arraysize(thread_handles); ++i) { HANDLE pipe = CreateNamedPipe(pipe_name_16.c_str(), @@ -311,7 +311,7 @@ void ExceptionHandlerServer::Run(Delegate* delegate, message.type = ClientToServerMessage::kShutdown; message.shutdown.token = shutdown_token; ServerToClientMessage response; - SendToCrashHandlerServer(base::UTF8ToUTF16(pipe_name), + SendToCrashHandlerServer(pipe_name_16, reinterpret_cast(message), &response); } diff --git a/util/win/exception_handler_server.h b/util/win/exception_handler_server.h index 6928a6d9..d1243406 100644 --- a/util/win/exception_handler_server.h +++ b/util/win/exception_handler_server.h @@ -61,16 +61,18 @@ class ExceptionHandlerServer { }; //! \brief Constructs the exception handling server. - ExceptionHandlerServer(); + //! + //! \param[in] pipe_name The name of the pipe to listen on. Must be of the + //! form "\\.\pipe\". + explicit ExceptionHandlerServer(const std::string& pipe_name); + ~ExceptionHandlerServer(); //! \brief Runs the exception-handling server. //! //! \param[in] delegate The interface to which the exceptions are delegated //! when they are caught in Run(). Ownership is not transferred. - //! \param[in] pipe_name The name of the pipe to listen on. Must be of the - //! form "\\.\pipe\". - void Run(Delegate* delegate, const std::string& pipe_name); + void Run(Delegate* delegate); //! \brief Stops the exception-handling server. Returns immediately. The //! object must not be destroyed until Run() returns. @@ -84,6 +86,7 @@ class ExceptionHandlerServer { static void __stdcall OnNonCrashDumpEvent(void* ctx, BOOLEAN); static void __stdcall OnProcessEnd(void* ctx, BOOLEAN); + std::string pipe_name_; ScopedKernelHANDLE port_; base::Lock clients_lock_; diff --git a/util/win/exception_handler_server_test.cc b/util/win/exception_handler_server_test.cc index 0d87086b..9fe593b5 100644 --- a/util/win/exception_handler_server_test.cc +++ b/util/win/exception_handler_server_test.cc @@ -36,20 +36,18 @@ 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, - const std::string& pipe_name) - : server_(server), delegate_(delegate), pipe_name_(pipe_name) {} + ExceptionHandlerServer::Delegate* delegate) + : server_(server), delegate_(delegate) {} ~RunServerThread() override {} private: // Thread: - void ThreadMain() override { server_->Run(delegate_, pipe_name_); } + void ThreadMain() override { server_->Run(delegate_); } ExceptionHandlerServer* server_; ExceptionHandlerServer::Delegate* delegate_; - std::string pipe_name_; DISALLOW_COPY_AND_ASSIGN(RunServerThread); }; @@ -85,8 +83,8 @@ class ExceptionHandlerServerTest : public testing::Test { base::StringPrintf("%08x", GetCurrentProcessId())), server_ready_(CreateEvent(nullptr, false, false, nullptr)), delegate_(server_ready_.get()), - server_(), - server_thread_(&server_, &delegate_, pipe_name_) {} + server_(pipe_name_), + server_thread_(&server_, &delegate_) {} TestDelegate& delegate() { return delegate_; } ExceptionHandlerServer& server() { return server_; }