From cae59b4ae44ccc667be4c28710d167ad4dd9823b Mon Sep 17 00:00:00 2001 From: "ted.mielczarek@gmail.com" Date: Wed, 15 Dec 2010 16:28:22 +0000 Subject: [PATCH] issue 334 - Fix a race condition between ExceptionHandler::Teardown and ExceptionHandler::WaitForMessage on OS X R=mark at http://breakpad.appspot.com/165001/show git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@744 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/client/mac/handler/exception_handler.cc | 15 +++++++++++---- src/client/mac/handler/exception_handler.h | 5 +++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/client/mac/handler/exception_handler.cc b/src/client/mac/handler/exception_handler.cc index 89cc4e90..b9f84b48 100644 --- a/src/client/mac/handler/exception_handler.cc +++ b/src/client/mac/handler/exception_handler.cc @@ -53,6 +53,9 @@ namespace google_breakpad { using std::map; +// Message ID telling the handler thread to quit. +static const mach_msg_id_t kQuitMessage = 1; + // These structures and techniques are illustrated in // Mac OS X Internals, Amit Singh, ch 9.7 struct ExceptionMessage { @@ -281,7 +284,7 @@ bool ExceptionHandler::WriteMinidump() { if (pthread_mutex_lock(&minidump_write_mutex_) == 0) { // Send an empty message to the handle port so that a minidump will // be written - SendEmptyMachMessage(); + SendEmptyMachMessage(false); // Wait for the minidump writer to complete its writing. It will unlock // the mutex when completed @@ -522,7 +525,9 @@ void *ExceptionHandler::WaitForMessage(void *exception_handler_class) { // to avoid misleading stacks. If appropriate they will be resumed // afterwards. if (!receive.exception) { - if (self->is_in_teardown_) + // Don't touch self, since this message could have been sent + // from its destructor. + if (receive.header.msgh_id == kQuitMessage) return NULL; self->SuspendThreads(); @@ -705,7 +710,7 @@ bool ExceptionHandler::Teardown() { return false; // Send an empty message so that the handler_thread exits - if (SendEmptyMachMessage()) { + if (SendEmptyMachMessage(true)) { mach_port_t current_task = mach_task_self(); result = mach_port_deallocate(current_task, handler_port_); if (result != KERN_SUCCESS) @@ -721,9 +726,11 @@ bool ExceptionHandler::Teardown() { return result == KERN_SUCCESS; } -bool ExceptionHandler::SendEmptyMachMessage() { +bool ExceptionHandler::SendEmptyMachMessage(bool quit) { ExceptionMessage empty; memset(&empty, 0, sizeof(empty)); + if (quit) + empty.header.msgh_id = kQuitMessage; empty.header.msgh_size = sizeof(empty) - sizeof(empty.padding); empty.header.msgh_remote_port = handler_port_; empty.header.msgh_bits = MACH_MSGH_BITS(MACH_MSG_TYPE_COPY_SEND, diff --git a/src/client/mac/handler/exception_handler.h b/src/client/mac/handler/exception_handler.h index 76353255..182faa9d 100644 --- a/src/client/mac/handler/exception_handler.h +++ b/src/client/mac/handler/exception_handler.h @@ -150,8 +150,9 @@ class ExceptionHandler { bool Teardown(); // Send an "empty" mach message to the exception handler. Return true on - // success, false otherwise - bool SendEmptyMachMessage(); + // success, false otherwise. If quit is true, the handler thread should + // simply quit. + bool SendEmptyMachMessage(bool quit); // All minidump writing goes through this one routine bool WriteMinidumpWithException(int exception_type, int exception_code,