From: Tudor Bosman <tudorb@fb.com>
Date: Thu, 12 Dec 2013 23:08:05 +0000 (-0800)
Subject: Fix test, also do not read from stack of another thread
X-Git-Tag: v0.22.0~759
X-Git-Url: http://plrg.eecs.uci.edu/git/?p=folly.git;a=commitdiff_plain;h=3aed59d8e656abb09c33102f7a7caf45cf06155d;ds=sidebyside

Fix test, also do not read from stack of another thread

Summary:
... even though Google's signal handler does it
(https://code.google.com/p/google-glog/source/browse/trunk/src/signalhandler.cc?r=16#235)

Assuming the existence of an invalid pthread_t is a lesser evil than reading
from another thread's stack, IMO.

Test Plan: folly/experimental/symbolizer/test

Reviewed By: simpkins@fb.com

FB internal diff: D1096620
---

diff --git a/folly/experimental/symbolizer/SignalHandler.cpp b/folly/experimental/symbolizer/SignalHandler.cpp
index d19cc973..8346e79b 100644
--- a/folly/experimental/symbolizer/SignalHandler.cpp
+++ b/folly/experimental/symbolizer/SignalHandler.cpp
@@ -206,16 +206,22 @@ void dumpStackTrace() {
   }
 }
 
-std::atomic<pthread_t*> gSignalThread;
+// On Linux, pthread_t is a pointer, so 0 is an invalid value, which we
+// take to indicate "no thread in the signal handler".
+//
+// POSIX defines PTHREAD_NULL for this purpose, but that's not available.
+constexpr pthread_t kInvalidThreadId = 0;
+
+std::atomic<pthread_t> gSignalThread(kInvalidThreadId);
 
 // Here be dragons.
 void innerSignalHandler(int signum, siginfo_t* info, void* uctx) {
   // First, let's only let one thread in here at a time.
   pthread_t myId = pthread_self();
 
-  pthread_t* prevSignalThread = nullptr;
-  while (!gSignalThread.compare_exchange_strong(prevSignalThread, &myId)) {
-    if (pthread_equal(*prevSignalThread, myId)) {
+  pthread_t prevSignalThread = kInvalidThreadId;
+  while (!gSignalThread.compare_exchange_strong(prevSignalThread, myId)) {
+    if (pthread_equal(prevSignalThread, myId)) {
       print("Entered fatal signal handler recursively. We're in trouble.\n");
       return;
     }
@@ -226,7 +232,7 @@ void innerSignalHandler(int signum, siginfo_t* info, void* uctx) {
     ts.tv_nsec = 100L * 1000 * 1000;  // 100ms
     nanosleep(&ts, nullptr);
 
-    prevSignalThread = nullptr;
+    prevSignalThread = kInvalidThreadId;
   }
 
   dumpTimeInfo();
@@ -241,7 +247,7 @@ void signalHandler(int signum, siginfo_t* info, void* uctx) {
   SCOPE_EXIT { fsyncNoInt(STDERR_FILENO); };
   innerSignalHandler(signum, info, uctx);
 
-  gSignalThread = nullptr;
+  gSignalThread = kInvalidThreadId;
   // Kill ourselves with the previous handler.
   callPreviousSignalHandler(signum);
 }
diff --git a/folly/experimental/symbolizer/test/SignalHandlerTest.cpp b/folly/experimental/symbolizer/test/SignalHandlerTest.cpp
index 89f81591..db58708d 100644
--- a/folly/experimental/symbolizer/test/SignalHandlerTest.cpp
+++ b/folly/experimental/symbolizer/test/SignalHandlerTest.cpp
@@ -45,6 +45,7 @@ TEST(SignalHandler, Simple) {
   addFatalSignalCallback(callback1);
   addFatalSignalCallback(callback2);
   installFatalSignalHandler();
+  installFatalSignalCallbacks();
 
   EXPECT_DEATH(
       failHard(),