From 28970fab19de7e975e7f25addba9ff5267616929 Mon Sep 17 00:00:00 2001 From: "mark@chromium.org" Date: Tue, 24 Jul 2012 20:33:12 +0000 Subject: [PATCH] Back out r996, reinstating r985. That wasn't it. git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@997 4c0a9323-5329-0410-9bdc-e9ce6186880e --- .../crash_generation_client.cc | 56 +++++++++++++++++++ .../crash_generation_client.h | 16 ++++++ .../windows/handler/exception_handler.cc | 38 ++++++++++++- .../windows/handler/exception_handler.h | 14 ++++- 4 files changed, 120 insertions(+), 4 deletions(-) diff --git a/src/client/windows/crash_generation/crash_generation_client.cc b/src/client/windows/crash_generation/crash_generation_client.cc index ffbcaf20..b0d3d04d 100644 --- a/src/client/windows/crash_generation/crash_generation_client.cc +++ b/src/client/windows/crash_generation/crash_generation_client.cc @@ -95,6 +95,27 @@ CrashGenerationClient::CrashGenerationClient( MINIDUMP_TYPE dump_type, const CustomClientInfo* custom_info) : pipe_name_(pipe_name), + pipe_handle_(NULL), + dump_type_(dump_type), + thread_id_(0), + server_process_id_(0), + crash_event_(NULL), + crash_generated_(NULL), + server_alive_(NULL), + exception_pointers_(NULL), + custom_info_() { + memset(&assert_info_, 0, sizeof(assert_info_)); + if (custom_info) { + custom_info_ = *custom_info; + } +} + +CrashGenerationClient::CrashGenerationClient( + HANDLE pipe_handle, + MINIDUMP_TYPE dump_type, + const CustomClientInfo* custom_info) + : pipe_name_(), + pipe_handle_(pipe_handle), dump_type_(dump_type), thread_id_(0), server_process_id_(0), @@ -248,6 +269,12 @@ bool CrashGenerationClient::RegisterClient(HANDLE pipe) { HANDLE CrashGenerationClient::ConnectToPipe(const wchar_t* pipe_name, DWORD pipe_access, DWORD flags_attrs) { + if (pipe_handle_) { + HANDLE t = pipe_handle_; + pipe_handle_ = NULL; + return t; + } + for (int i = 0; i < kPipeConnectMaxAttempts; ++i) { HANDLE pipe = CreateFile(pipe_name, pipe_access, @@ -342,4 +369,33 @@ bool CrashGenerationClient::SignalCrashEventAndWait() { return result == WAIT_OBJECT_0; } +HANDLE CrashGenerationClient::DuplicatePipeToClientProcess(const wchar_t* pipe_name, + HANDLE hProcess) { + for (int i = 0; i < kPipeConnectMaxAttempts; ++i) { + HANDLE local_pipe = CreateFile(pipe_name, kPipeDesiredAccess, + 0, NULL, OPEN_EXISTING, + kPipeFlagsAndAttributes, NULL); + if (local_pipe != INVALID_HANDLE_VALUE) { + HANDLE remotePipe = INVALID_HANDLE_VALUE; + if (DuplicateHandle(GetCurrentProcess(), local_pipe, + hProcess, &remotePipe, 0, FALSE, + DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS)) { + return remotePipe; + } else { + return INVALID_HANDLE_VALUE; + } + } + + // Cannot continue retrying if the error wasn't a busy pipe. + if (GetLastError() != ERROR_PIPE_BUSY) { + return INVALID_HANDLE_VALUE; + } + + if (!WaitNamedPipe(pipe_name, kPipeBusyWaitTimeoutMs)) { + return INVALID_HANDLE_VALUE; + } + } + return INVALID_HANDLE_VALUE; +} + } // namespace google_breakpad diff --git a/src/client/windows/crash_generation/crash_generation_client.h b/src/client/windows/crash_generation/crash_generation_client.h index 85a0456c..2ce14dd2 100644 --- a/src/client/windows/crash_generation/crash_generation_client.h +++ b/src/client/windows/crash_generation/crash_generation_client.h @@ -66,6 +66,10 @@ class CrashGenerationClient { MINIDUMP_TYPE dump_type, const CustomClientInfo* custom_info); + CrashGenerationClient(HANDLE pipe_handle, + MINIDUMP_TYPE dump_type, + const CustomClientInfo* custom_info); + ~CrashGenerationClient(); // Registers the client process with the crash server. @@ -96,6 +100,14 @@ class CrashGenerationClient { // false will be returned. bool RequestDump(MDRawAssertionInfo* assert_info); + // If the crash generation client is running in a sandbox that prevents it + // from opening the named pipe directly, the server process may open the + // handle and duplicate it into the client process with this helper method. + // Returns INVALID_HANDLE_VALUE on failure. The process must have been opened + // with the PROCESS_DUP_HANDLE access right. + static HANDLE DuplicatePipeToClientProcess(const wchar_t* pipe_name, + HANDLE hProcess); + private: // Connects to the appropriate pipe and sets the pipe handle state. // @@ -127,6 +139,10 @@ class CrashGenerationClient { // Pipe name to use to talk to server. std::wstring pipe_name_; + // Pipe handle duplicated from server process. Only valid before + // Register is called. + HANDLE pipe_handle_; + // Custom client information CustomClientInfo custom_info_; diff --git a/src/client/windows/handler/exception_handler.cc b/src/client/windows/handler/exception_handler.cc index 386d8557..9ede97f0 100755 --- a/src/client/windows/handler/exception_handler.cc +++ b/src/client/windows/handler/exception_handler.cc @@ -69,9 +69,29 @@ ExceptionHandler::ExceptionHandler(const wstring& dump_path, handler_types, dump_type, pipe_name, + NULL, custom_info); } +ExceptionHandler::ExceptionHandler(const wstring& dump_path, + FilterCallback filter, + MinidumpCallback callback, + void* callback_context, + int handler_types, + MINIDUMP_TYPE dump_type, + HANDLE pipe_handle, + const CustomClientInfo* custom_info) { + Initialize(dump_path, + filter, + callback, + callback_context, + handler_types, + dump_type, + NULL, + pipe_handle, + custom_info); +} + ExceptionHandler::ExceptionHandler(const wstring &dump_path, FilterCallback filter, MinidumpCallback callback, @@ -84,6 +104,7 @@ ExceptionHandler::ExceptionHandler(const wstring &dump_path, handler_types, MiniDumpNormal, NULL, + NULL, NULL); } @@ -94,6 +115,7 @@ void ExceptionHandler::Initialize(const wstring& dump_path, int handler_types, MINIDUMP_TYPE dump_type, const wchar_t* pipe_name, + HANDLE pipe_handle, const CustomClientInfo* custom_info) { LONG instance_count = InterlockedIncrement(&instance_count_); filter_ = filter; @@ -123,12 +145,22 @@ void ExceptionHandler::Initialize(const wstring& dump_path, handler_return_value_ = false; handle_debug_exceptions_ = false; - // Attempt to use out-of-process if user has specified pipe name. - if (pipe_name != NULL) { - scoped_ptr client( + // Attempt to use out-of-process if user has specified a pipe. + if (pipe_name != NULL || pipe_handle != NULL) { + assert(!(pipe_name && pipe_handle)); + + scoped_ptr client; + if (pipe_name) { + client.reset( new CrashGenerationClient(pipe_name, dump_type_, custom_info)); + } else { + client.reset( + new CrashGenerationClient(pipe_handle, + dump_type_, + custom_info)); + } // If successful in registering with the monitoring process, // there is no need to setup in-process crash generation. diff --git a/src/client/windows/handler/exception_handler.h b/src/client/windows/handler/exception_handler.h index ce3bcb0d..38c2c3ca 100644 --- a/src/client/windows/handler/exception_handler.h +++ b/src/client/windows/handler/exception_handler.h @@ -164,7 +164,7 @@ class ExceptionHandler { void* callback_context, int handler_types); - // Creates a new ExcetpionHandler instance that can attempt to perform + // Creates a new ExceptionHandler instance that can attempt to perform // out-of-process dump generation if pipe_name is not NULL. If pipe_name is // NULL, or if out-of-process dump generation registration step fails, // in-process dump generation will be used. This also allows specifying @@ -178,6 +178,17 @@ class ExceptionHandler { const wchar_t* pipe_name, const CustomClientInfo* custom_info); + // As above, creates a new ExceptionHandler instance to perform + // out-of-process dump generation if the given pipe_handle is not NULL. + ExceptionHandler(const wstring& dump_path, + FilterCallback filter, + MinidumpCallback callback, + void* callback_context, + int handler_types, + MINIDUMP_TYPE dump_type, + HANDLE pipe_handle, + const CustomClientInfo* custom_info); + ~ExceptionHandler(); // Get and set the minidump path. @@ -235,6 +246,7 @@ class ExceptionHandler { int handler_types, MINIDUMP_TYPE dump_type, const wchar_t* pipe_name, + HANDLE pipe_handle, const CustomClientInfo* custom_info); // Function pointer type for MiniDumpWriteDump, which is looked up