From 02c244f1b29baf8dd519a1175fb19df0cdb99a0f Mon Sep 17 00:00:00 2001 From: nealsid Date: Thu, 26 Feb 2009 21:31:53 +0000 Subject: [PATCH] Fix for issue 242, plus a redo of how old exception handlers are tracked, and called. R=Craig.Schlenter, luly81 git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@315 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/client/linux/handler/Makefile | 2 +- src/client/linux/handler/exception_handler.cc | 62 ++++++++++++++----- src/client/linux/handler/exception_handler.h | 17 +++-- 3 files changed, 59 insertions(+), 22 deletions(-) diff --git a/src/client/linux/handler/Makefile b/src/client/linux/handler/Makefile index ef30df1d..73be56d8 100644 --- a/src/client/linux/handler/Makefile +++ b/src/client/linux/handler/Makefile @@ -1,7 +1,7 @@ CXX=g++ CC=gcc -CXXFLAGS=-gstabs -I../../.. -Wall -D_REENTRANT +CXXFLAGS=-gstabs+ -I../../.. -Wall -D_REENTRANT LDFLAGS=-lpthread OBJ_DIR=. diff --git a/src/client/linux/handler/exception_handler.cc b/src/client/linux/handler/exception_handler.cc index b253cb39..26e09b7d 100644 --- a/src/client/linux/handler/exception_handler.cc +++ b/src/client/linux/handler/exception_handler.cc @@ -81,6 +81,15 @@ ExceptionHandler::ExceptionHandler(const string &dump_path, installed_handler_(install_handler) { set_dump_path(dump_path); + act_.sa_handler = HandleException; + act_.sa_flags = SA_ONSTACK; + sigemptyset(&act_.sa_mask); + // now, make sure we're blocking all the signals we are handling + // when we're handling any of them + for ( size_t i = 0; i < sizeof(SigTable) / sizeof(SigTable[0]); ++i) { + sigaddset(&act_.sa_mask, SigTable[i]); + } + if (install_handler) { SetupHandler(); pthread_mutex_lock(&handler_stack_mutex_); @@ -149,20 +158,26 @@ void ExceptionHandler::SetupHandler() { } void ExceptionHandler::SetupHandler(int signo) { - struct sigaction act, old_act; - act.sa_handler = HandleException; - act.sa_flags = SA_ONSTACK; - if (sigaction(signo, &act, &old_act) < 0) - return; - old_handlers_[signo] = old_act.sa_handler; + + // We're storing pointers to the old signal action + // structure, rather than copying the structure + // because we can't count on the sa_mask field to + // be scalar. + struct sigaction *old_act = &old_actions_[signo]; + + if (sigaction(signo, &act_, old_act) < 0) + return; } void ExceptionHandler::TeardownHandler(int signo) { - if (old_handlers_.find(signo) != old_handlers_.end()) { - struct sigaction act; - act.sa_handler = old_handlers_[signo]; - act.sa_flags = 0; - sigaction(signo, &act, 0); + TeardownHandler(signo, NULL); +} + +void ExceptionHandler::TeardownHandler(int signo, struct sigaction *final_handler) { + if (old_actions_[signo].sa_handler) { + struct sigaction *act = &old_actions_[signo]; + sigaction(signo, act, final_handler); + memset(&old_actions_[signo], 0x0, sizeof(struct sigaction)); } } @@ -193,7 +208,8 @@ void ExceptionHandler::HandleException(int signo) { pthread_mutex_unlock(&handler_stack_mutex_); // Restore original handler. - current_handler->TeardownHandler(signo); + struct sigaction old_action; + current_handler->TeardownHandler(signo, &old_action); struct sigcontext *sig_ctx = NULL; if (current_handler->InternalWriteMinidump(signo, current_ebp, &sig_ctx)) { @@ -202,11 +218,23 @@ void ExceptionHandler::HandleException(int signo) { } else { // Exception not fully handled, will call the next handler in stack to // process it. - typedef void (*SignalHandler)(int signo, struct sigcontext); - SignalHandler old_handler = - reinterpret_cast(current_handler->old_handlers_[signo]); - if (old_handler != NULL && sig_ctx != NULL) + if (old_action.sa_handler != NULL && sig_ctx != NULL) { + + // Have our own typedef, because of the comment above w.r.t signal + // context on the stack + typedef void (*SignalHandler)(int signo, struct sigcontext); + + SignalHandler old_handler = + reinterpret_cast(old_action.sa_handler); + + sigset_t old_set; + // Use SIG_BLOCK here because we don't want to unblock a signal + // that the signal handler we're currently in needs to block + sigprocmask(SIG_BLOCK, &old_action.sa_mask, &old_set); old_handler(signo, *sig_ctx); + sigprocmask(SIG_SETMASK, &old_set, NULL); + } + } pthread_mutex_lock(&handler_stack_mutex_); @@ -247,7 +275,7 @@ bool ExceptionHandler::InternalWriteMinidump(int signo, // Unblock the signals. if (blocked) { - sigprocmask(SIG_SETMASK, &sig_old, &sig_old); + sigprocmask(SIG_SETMASK, &sig_old, NULL); } if (callback_) diff --git a/src/client/linux/handler/exception_handler.h b/src/client/linux/handler/exception_handler.h index 6e6156c5..6ea09a11 100644 --- a/src/client/linux/handler/exception_handler.h +++ b/src/client/linux/handler/exception_handler.h @@ -36,6 +36,7 @@ #include #include +#include #include #include "client/linux/handler/minidump_generator.h" @@ -146,6 +147,8 @@ class ExceptionHandler { void SetupHandler(int signo); // Teardown the handler for a signal. void TeardownHandler(int signo); + // Teardown the handler for a signal. + void TeardownHandler(int signo, struct sigaction *old); // Teardown all handlers. void TeardownAllHandler(); @@ -192,10 +195,6 @@ class ExceptionHandler { // when created (with an install_handler parameter set to true). bool installed_handler_; - // Keep the previous handlers for the signal. - typedef void (*sighandler_t)(int); - std::map old_handlers_; - // The global exception handler stack. This is need becuase there may exist // multiple ExceptionHandler instances in a process. Each will have itself // registered in this stack. @@ -210,6 +209,16 @@ class ExceptionHandler { // disallow copy ctor and operator= explicit ExceptionHandler(const ExceptionHandler &); void operator=(const ExceptionHandler &); + + // The sigactions structure we use for each signal + struct sigaction act_; + + + // Keep the previous handlers for the signal. + // We're wasting a bit of memory here since we only change + // the handler for some signals but i want to avoid allocating + // memory in the signal handler + struct sigaction old_actions_[NSIG]; }; } // namespace google_breakpad