From febb444dcd9ff1bba82e31bd76ae529eee4fb5c4 Mon Sep 17 00:00:00 2001 From: "mark@chromium.org" Date: Tue, 24 Jul 2012 19:36:34 +0000 Subject: Speculatively back out r985 because it may be causing crash_service problems for Chrome. See http://codereview.chromium.org/10805065/ . I'll recommit this if it wasn't the problem. git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@996 4c0a9323-5329-0410-9bdc-e9ce6186880e --- .../crash_generation/crash_generation_client.cc | 56 ---------------------- .../crash_generation/crash_generation_client.h | 16 ------- src/client/windows/handler/exception_handler.cc | 38 ++------------- src/client/windows/handler/exception_handler.h | 14 +----- 4 files changed, 4 insertions(+), 120 deletions(-) diff --git a/src/client/windows/crash_generation/crash_generation_client.cc b/src/client/windows/crash_generation/crash_generation_client.cc index b0d3d04d..ffbcaf20 100644 --- a/src/client/windows/crash_generation/crash_generation_client.cc +++ b/src/client/windows/crash_generation/crash_generation_client.cc @@ -95,27 +95,6 @@ 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), @@ -269,12 +248,6 @@ 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, @@ -369,33 +342,4 @@ 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 2ce14dd2..85a0456c 100644 --- a/src/client/windows/crash_generation/crash_generation_client.h +++ b/src/client/windows/crash_generation/crash_generation_client.h @@ -66,10 +66,6 @@ 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. @@ -100,14 +96,6 @@ 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. // @@ -139,10 +127,6 @@ 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 9ede97f0..386d8557 100755 --- a/src/client/windows/handler/exception_handler.cc +++ b/src/client/windows/handler/exception_handler.cc @@ -69,29 +69,9 @@ 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, @@ -104,7 +84,6 @@ ExceptionHandler::ExceptionHandler(const wstring &dump_path, handler_types, MiniDumpNormal, NULL, - NULL, NULL); } @@ -115,7 +94,6 @@ 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; @@ -145,22 +123,12 @@ 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 a pipe. - if (pipe_name != NULL || pipe_handle != NULL) { - assert(!(pipe_name && pipe_handle)); - - scoped_ptr client; - if (pipe_name) { - client.reset( + // Attempt to use out-of-process if user has specified pipe name. + if (pipe_name != NULL) { + scoped_ptr client( 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 38c2c3ca..ce3bcb0d 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 ExceptionHandler instance that can attempt to perform + // Creates a new ExcetpionHandler 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,17 +178,6 @@ 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. @@ -246,7 +235,6 @@ 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 -- cgit v1.2.1