From: mtklein <mtklein@chromium.org>
Date: Thu, 9 Jun 2016 15:59:48 +0000 (-0700)
Subject: better exclusion for stack traces
X-Git-Tag: accepted/tizen/5.0/unified/20181102.025319~129^2~249
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5b64dab5bcb1823d465be1833db1f716f9b903e5;p=platform%2Fupstream%2FlibSkiaSharp.git

better exclusion for stack traces

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
---

diff --git a/dm/DM.cpp b/dm/DM.cpp
index de99b6b..9ddff28 100644
--- 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);
         }
     }