better exclusion for stack traces
authormtklein <mtklein@chromium.org>
Thu, 9 Jun 2016 15:59:48 +0000 (08:59 -0700)
committerCommit bot <commit-bot@chromium.org>
Thu, 9 Jun 2016 15:59:48 +0000 (08:59 -0700)
Instead of two synchronization systems (in_signal_handler, gMutex),
we can just use one.  This simplifies the signal handler logic to:
   - first thread through grabs the lock, prints what's running and a stack trace,
     then exits
   - all other threads just sit waiting on that lock until exit kills them

Previously I think all threads were racing to exit, which can kill the process
before the printing thread is done.  That truncated the output, which is dumb.

Plus...
   refactor slightly so that crash_handler() shows up at the top of the stack
   trace rather than some odd name for a lambda inside setup_crash_handler().

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2051863002

Review-Url: https://codereview.chromium.org/2051863002

dm/DM.cpp

index de99b6b3e226a688ac3f8c60477a4e9aef0fe941..9ddff282f386f6b2ec300454c8e1eb8829da682c 100644 (file)
--- a/dm/DM.cpp
+++ b/dm/DM.cpp
@@ -164,13 +164,8 @@ static void print_status() {
     }
 }
 
-// Yo dawg, I heard you like signals so I caught a signal in your
-// signal handler so you can handle signals while you handle signals.
-// Let's not get into that situation.  Only print if we're the first ones to get a crash signal.
-static std::atomic<bool> in_signal_handler{false};
-
 #if defined(SK_BUILD_FOR_WIN32)
-    static LONG WINAPI handler(EXCEPTION_POINTERS* e) {
+    static LONG WINAPI crash_handler(EXCEPTION_POINTERS* e) {
         static const struct {
             const char* name;
             DWORD code;
@@ -184,50 +179,55 @@ static std::atomic<bool> in_signal_handler{false};
         #undef _
         };
 
-        if (!in_signal_handler.exchange(true)) {
-            const DWORD code = e->ExceptionRecord->ExceptionCode;
-            info("\nCaught exception %u", code);
-            for (const auto& exception : kExceptions) {
-                if (exception.code == code) {
-                    info(" %s", exception.name);
-                }
+        SkAutoTAcquire<SkSpinlock> lock(gMutex);
+
+        const DWORD code = e->ExceptionRecord->ExceptionCode;
+        info("\nCaught exception %u", code);
+        for (const auto& exception : kExceptions) {
+            if (exception.code == code) {
+                info(" %s", exception.name);
             }
-            info("\n");
-            print_status();
-            fflush(stdout);
         }
+        info(", was running:\n");
+        for (auto& task : gRunning) {
+            info("\t%s\n", task.c_str());
+        }
+        fflush(stdout);
+
         // Execute default exception handler... hopefully, exit.
         return EXCEPTION_EXECUTE_HANDLER;
     }
-    static void setup_crash_handler() { SetUnhandledExceptionFilter(handler); }
+    static void setup_crash_handler() { SetUnhandledExceptionFilter(crash_handler); }
 
 #elif !defined(SK_BUILD_FOR_ANDROID)
     #include <execinfo.h>
     #include <signal.h>
     #include <stdlib.h>
 
+    static void crash_handler(int sig) {
+        SkAutoTAcquire<SkSpinlock> lock(gMutex);
+
+        info("\nCaught signal %d [%s], was running:\n", sig, strsignal(sig));
+        for (auto& task : gRunning) {
+            info("\t%s\n", task.c_str());
+        }
+
+        void* stack[64];
+        int count = backtrace(stack, SK_ARRAY_COUNT(stack));
+        char** symbols = backtrace_symbols(stack, count);
+        info("\nStack trace:\n");
+        for (int i = 0; i < count; i++) {
+            info("    %s\n", symbols[i]);
+        }
+        fflush(stdout);
+
+        _Exit(sig);
+    }
+
     static void setup_crash_handler() {
         const int kSignals[] = { SIGABRT, SIGBUS, SIGFPE, SIGILL, SIGSEGV };
         for (int sig : kSignals) {
-            signal(sig, [](int sig) {
-                if (!in_signal_handler.exchange(true)) {
-                    SkAutoTAcquire<SkSpinlock> lock(gMutex);
-                    info("\nCaught signal %d [%s], was running:\n", sig, strsignal(sig));
-                    for (auto& task : gRunning) {
-                        info("\t%s\n", task.c_str());
-                    }
-
-                    void* stack[64];
-                    int count = backtrace(stack, SK_ARRAY_COUNT(stack));
-                    char** symbols = backtrace_symbols(stack, count);
-                    info("\nStack trace:\n");
-                    for (int i = 0; i < count; i++) {
-                        info("    %s\n", symbols[i]);
-                    }
-                    fflush(stdout);
-                }
-                _Exit(sig);
-            });
+            signal(sig, crash_handler);
         }
     }