From b26101995195dec32650a9d156f5a904130a49a6 Mon Sep 17 00:00:00 2001 From: mmentovai Date: Tue, 31 Oct 2006 16:49:38 +0000 Subject: Windows exception handler does not survive stack overflows (#34). r=brian, thanks also to darin - All minidump writing is now done on a dedicated thread. When a stack overflow exception occurs, the only work that needs to be done on the exception thread will easily fit within the guard page. http://groups.google.com/group/airbag-dev/browse_thread/thread/3935e339d8354a75 git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@57 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/client/windows/handler/exception_handler.cc | 97 +++++++++++++++++++++++-- src/client/windows/handler/exception_handler.h | 59 ++++++++++++++- 2 files changed, 145 insertions(+), 11 deletions(-) (limited to 'src/client') diff --git a/src/client/windows/handler/exception_handler.cc b/src/client/windows/handler/exception_handler.cc index e58dea65..1d64cf9e 100644 --- a/src/client/windows/handler/exception_handler.cc +++ b/src/client/windows/handler/exception_handler.cc @@ -37,6 +37,14 @@ namespace google_airbag { ExceptionHandler *ExceptionHandler::current_handler_ = NULL; +HANDLE ExceptionHandler::handler_thread_ = NULL; +CRITICAL_SECTION ExceptionHandler::handler_critical_section_; +HANDLE ExceptionHandler::handler_start_semaphore_ = NULL; +HANDLE ExceptionHandler::handler_finish_semaphore_ = NULL; +ExceptionHandler *ExceptionHandler::requesting_handler_ = NULL; +DWORD ExceptionHandler::requesting_thread_id_ = 0; +EXCEPTION_POINTERS *ExceptionHandler::exception_info_ = NULL; +bool ExceptionHandler::handler_return_value_ = false; ExceptionHandler::ExceptionHandler(const wstring &dump_path, MinidumpCallback callback, @@ -46,6 +54,22 @@ ExceptionHandler::ExceptionHandler(const wstring &dump_path, dump_path_(dump_path), dbghelp_module_(NULL), minidump_write_dump_(NULL), previous_handler_(current_handler_), previous_filter_(NULL) { + if (!handler_thread_) { + // The first time an ExceptionHandler is created, set up the handler + // thread and the synchronization primitives. + InitializeCriticalSection(&handler_critical_section_); + handler_start_semaphore_ = CreateSemaphore(NULL, 0, 1, NULL); + handler_finish_semaphore_ = CreateSemaphore(NULL, 0, 1, NULL); + + DWORD thread_id; + handler_thread_ = CreateThread(NULL, // lpThreadAttributes + 64 * 1024, // dwStackSize + ExceptionHandlerThreadMain, + NULL, // lpParameter + 0, // dwCreationFlags + &thread_id); + } + UpdateNextID(); dbghelp_module_ = LoadLibrary(L"dbghelp.dll"); if (dbghelp_module_) { @@ -66,18 +90,72 @@ ExceptionHandler::~ExceptionHandler() { SetUnhandledExceptionFilter(previous_filter_); current_handler_ = previous_handler_; } + + if (previous_handler_ == NULL) { + // When destroying the last ExceptionHandler, clean up the handler thread + // and synchronization primitives. + TerminateThread(handler_thread_, 1); + handler_thread_ = NULL; + DeleteCriticalSection(&handler_critical_section_); + CloseHandle(handler_start_semaphore_); + handler_start_semaphore_ = NULL; + CloseHandle(handler_finish_semaphore_); + handler_finish_semaphore_ = NULL; + } } // static -LONG ExceptionHandler::HandleException(EXCEPTION_POINTERS *exinfo) { - if (!current_handler_->WriteMinidumpWithException(exinfo)) { - return EXCEPTION_CONTINUE_SEARCH; +DWORD ExceptionHandler::ExceptionHandlerThreadMain(void *lpParameter) { + while (true) { + if (WaitForSingleObject(handler_start_semaphore_, INFINITE) == + WAIT_OBJECT_0) { + // Perform the requested action. + handler_return_value_ = requesting_handler_->WriteMinidumpWithException( + requesting_thread_id_, exception_info_); + + // Allow the requesting thread to proceed. + ReleaseSemaphore(handler_finish_semaphore_, 1, NULL); + } } - return EXCEPTION_EXECUTE_HANDLER; + + // Not reached. This thread will be terminated by ExceptionHandler's + // destructor. + return 0; +} + +// static +LONG ExceptionHandler::HandleException(EXCEPTION_POINTERS *exinfo) { + return current_handler_->WriteMinidumpOnHandlerThread(exinfo) ? + EXCEPTION_EXECUTE_HANDLER : EXCEPTION_CONTINUE_SEARCH; +} + +bool ExceptionHandler::WriteMinidumpOnHandlerThread(EXCEPTION_POINTERS *exinfo) { + EnterCriticalSection(&handler_critical_section_); + + // Set up data to be passed in to the handler thread. + requesting_handler_ = this; + requesting_thread_id_ = GetCurrentThreadId(); + exception_info_ = exinfo; + + // This causes the handler thread to call WriteMinidumpWithException. + ReleaseSemaphore(handler_start_semaphore_, 1, NULL); + + // Wait until WriteMinidumpWithException is done and collect its return value. + WaitForSingleObject(handler_finish_semaphore_, INFINITE); + bool status = handler_return_value_; + + // Clean up. + requesting_handler_ = NULL; + requesting_thread_id_ = 0; + exception_info_ = NULL; + + LeaveCriticalSection(&handler_critical_section_); + + return status; } bool ExceptionHandler::WriteMinidump() { - bool success = WriteMinidumpWithException(NULL); + bool success = WriteMinidumpOnHandlerThread(NULL); UpdateNextID(); return success; } @@ -90,7 +168,8 @@ bool ExceptionHandler::WriteMinidump(const wstring &dump_path, return handler.WriteMinidump(); } -bool ExceptionHandler::WriteMinidumpWithException(EXCEPTION_POINTERS *exinfo) { +bool ExceptionHandler::WriteMinidumpWithException(DWORD requesting_thread_id, + EXCEPTION_POINTERS *exinfo) { wchar_t dump_file_name[MAX_PATH]; swprintf_s(dump_file_name, MAX_PATH, L"%s\\%s.dmp", dump_path_.c_str(), next_minidump_id_.c_str()); @@ -106,16 +185,18 @@ bool ExceptionHandler::WriteMinidumpWithException(EXCEPTION_POINTERS *exinfo) { NULL); if (dump_file != INVALID_HANDLE_VALUE) { MINIDUMP_EXCEPTION_INFORMATION except_info; - except_info.ThreadId = GetCurrentThreadId(); + except_info.ThreadId = requesting_thread_id; except_info.ExceptionPointers = exinfo; except_info.ClientPointers = FALSE; + // TODO(mmentovai): include IDs of handler and requesting threads. + // The explicit comparison to TRUE avoids a warning (C4800). success = (minidump_write_dump_(GetCurrentProcess(), GetCurrentProcessId(), dump_file, MiniDumpNormal, - &except_info, + exinfo ? &except_info : NULL, NULL, NULL) == TRUE); diff --git a/src/client/windows/handler/exception_handler.h b/src/client/windows/handler/exception_handler.h index 9f7f14d0..b49093aa 100644 --- a/src/client/windows/handler/exception_handler.h +++ b/src/client/windows/handler/exception_handler.h @@ -116,12 +116,30 @@ class ExceptionHandler { CONST PMINIDUMP_USER_STREAM_INFORMATION UserStreamParam, CONST PMINIDUMP_CALLBACK_INFORMATION CallbackParam); - // This function does the actual writing of a minidump. - bool WriteMinidumpWithException(EXCEPTION_POINTERS *exinfo); + // Runs the main loop for the exception handler thread. + static DWORD WINAPI ExceptionHandlerThreadMain(void *lpParameter); - // Called when an unhandled exception occurs. + // Called on the exception thread when an unhandled exception occurs. + // Signals the exception handler thread to handle the exception. static LONG WINAPI HandleException(EXCEPTION_POINTERS *exinfo); + // This is called on the exception thread or on another thread that + // the user wishes to produce a dump from. It calls + // WriteMinidumpWithException on the handler thread, avoiding stack + // overflows and inconsistent dumps due to writing the dump from + // the exception thread. If the dump is requested as a result of an + // exception, exinfo contains exception information, otherwise, it + // is NULL. + bool WriteMinidumpOnHandlerThread(EXCEPTION_POINTERS *exinfo); + + // This function does the actual writing of a minidump. It is called + // on the handler thread. requesting_thread_id is the ID of the thread + // that requested the dump. If the dump is requested as a result of + // an exception, exinfo contains exception information, otherwise, + // it is NULL. + bool WriteMinidumpWithException(DWORD requesting_thread_id, + EXCEPTION_POINTERS *exinfo); + // Generates a new ID and stores it in next_minidump_id_. void UpdateNextID(); @@ -140,6 +158,41 @@ class ExceptionHandler { // the currently-installed ExceptionHandler, of which there can be only 1 static ExceptionHandler *current_handler_; + // The exception handler thread, if one has been created. + static HANDLE handler_thread_; + + // The critical section enforcing the requirement that only one exception be + // handled at a time. + static CRITICAL_SECTION handler_critical_section_; + + // Semaphores used to move exception handling between the exception thread + // and the handler thread. handler_start_semaphore_ is signalled by the + // exception thread to wake up the handler thread when an exception occurs. + // handler_finish_semaphore_ is signalled by the handler thread to wake up + // the exception thread when handling is complete. + static HANDLE handler_start_semaphore_; + static HANDLE handler_finish_semaphore_; + + // The next 3 fields are static data passed from the requesting thread to + // the handler thread. + + // The ExceptionHandler through which a request to write a dump was routed. + // This will be the same as current_handler_ for exceptions, but + // user-requested dumps may be routed through any live ExceptionHandler. + static ExceptionHandler *requesting_handler_; + + // The thread ID of the thread requesting the dump (either the exception + // thread or any other thread that called WriteMinidump directly). + static DWORD requesting_thread_id_; + + // The exception info passed to the exception handler on the exception + // thread, if an exception occurred. NULL for user-requested dumps. + static EXCEPTION_POINTERS *exception_info_; + + // The return value of the handler, passed from the handler thread back to + // the requesting thread. + static bool handler_return_value_; + // disallow copy ctor and operator= explicit ExceptionHandler(const ExceptionHandler &); void operator=(const ExceptionHandler &); -- cgit v1.2.1