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
This commit is contained in:
ted.mielczarek@gmail.com 2010-12-15 16:28:22 +00:00
parent 7405ecd046
commit cae59b4ae4
2 changed files with 14 additions and 6 deletions

View File

@ -53,6 +53,9 @@ namespace google_breakpad {
using std::map; 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 // These structures and techniques are illustrated in
// Mac OS X Internals, Amit Singh, ch 9.7 // Mac OS X Internals, Amit Singh, ch 9.7
struct ExceptionMessage { struct ExceptionMessage {
@ -281,7 +284,7 @@ bool ExceptionHandler::WriteMinidump() {
if (pthread_mutex_lock(&minidump_write_mutex_) == 0) { if (pthread_mutex_lock(&minidump_write_mutex_) == 0) {
// Send an empty message to the handle port so that a minidump will // Send an empty message to the handle port so that a minidump will
// be written // be written
SendEmptyMachMessage(); SendEmptyMachMessage(false);
// Wait for the minidump writer to complete its writing. It will unlock // Wait for the minidump writer to complete its writing. It will unlock
// the mutex when completed // 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 // to avoid misleading stacks. If appropriate they will be resumed
// afterwards. // afterwards.
if (!receive.exception) { 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; return NULL;
self->SuspendThreads(); self->SuspendThreads();
@ -705,7 +710,7 @@ bool ExceptionHandler::Teardown() {
return false; return false;
// Send an empty message so that the handler_thread exits // Send an empty message so that the handler_thread exits
if (SendEmptyMachMessage()) { if (SendEmptyMachMessage(true)) {
mach_port_t current_task = mach_task_self(); mach_port_t current_task = mach_task_self();
result = mach_port_deallocate(current_task, handler_port_); result = mach_port_deallocate(current_task, handler_port_);
if (result != KERN_SUCCESS) if (result != KERN_SUCCESS)
@ -721,9 +726,11 @@ bool ExceptionHandler::Teardown() {
return result == KERN_SUCCESS; return result == KERN_SUCCESS;
} }
bool ExceptionHandler::SendEmptyMachMessage() { bool ExceptionHandler::SendEmptyMachMessage(bool quit) {
ExceptionMessage empty; ExceptionMessage empty;
memset(&empty, 0, sizeof(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_size = sizeof(empty) - sizeof(empty.padding);
empty.header.msgh_remote_port = handler_port_; empty.header.msgh_remote_port = handler_port_;
empty.header.msgh_bits = MACH_MSGH_BITS(MACH_MSG_TYPE_COPY_SEND, empty.header.msgh_bits = MACH_MSGH_BITS(MACH_MSG_TYPE_COPY_SEND,

View File

@ -150,8 +150,9 @@ class ExceptionHandler {
bool Teardown(); bool Teardown();
// Send an "empty" mach message to the exception handler. Return true on // Send an "empty" mach message to the exception handler. Return true on
// success, false otherwise // success, false otherwise. If quit is true, the handler thread should
bool SendEmptyMachMessage(); // simply quit.
bool SendEmptyMachMessage(bool quit);
// All minidump writing goes through this one routine // All minidump writing goes through this one routine
bool WriteMinidumpWithException(int exception_type, int exception_code, bool WriteMinidumpWithException(int exception_type, int exception_code,