Commit 3aed59d8 authored by Tudor Bosman's avatar Tudor Bosman Committed by Jordan DeLong

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
parent d6359188
...@@ -206,16 +206,22 @@ void dumpStackTrace() { ...@@ -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. // Here be dragons.
void innerSignalHandler(int signum, siginfo_t* info, void* uctx) { void innerSignalHandler(int signum, siginfo_t* info, void* uctx) {
// First, let's only let one thread in here at a time. // First, let's only let one thread in here at a time.
pthread_t myId = pthread_self(); pthread_t myId = pthread_self();
pthread_t* prevSignalThread = nullptr; pthread_t prevSignalThread = kInvalidThreadId;
while (!gSignalThread.compare_exchange_strong(prevSignalThread, &myId)) { while (!gSignalThread.compare_exchange_strong(prevSignalThread, myId)) {
if (pthread_equal(*prevSignalThread, myId)) { if (pthread_equal(prevSignalThread, myId)) {
print("Entered fatal signal handler recursively. We're in trouble.\n"); print("Entered fatal signal handler recursively. We're in trouble.\n");
return; return;
} }
...@@ -226,7 +232,7 @@ void innerSignalHandler(int signum, siginfo_t* info, void* uctx) { ...@@ -226,7 +232,7 @@ void innerSignalHandler(int signum, siginfo_t* info, void* uctx) {
ts.tv_nsec = 100L * 1000 * 1000; // 100ms ts.tv_nsec = 100L * 1000 * 1000; // 100ms
nanosleep(&ts, nullptr); nanosleep(&ts, nullptr);
prevSignalThread = nullptr; prevSignalThread = kInvalidThreadId;
} }
dumpTimeInfo(); dumpTimeInfo();
...@@ -241,7 +247,7 @@ void signalHandler(int signum, siginfo_t* info, void* uctx) { ...@@ -241,7 +247,7 @@ void signalHandler(int signum, siginfo_t* info, void* uctx) {
SCOPE_EXIT { fsyncNoInt(STDERR_FILENO); }; SCOPE_EXIT { fsyncNoInt(STDERR_FILENO); };
innerSignalHandler(signum, info, uctx); innerSignalHandler(signum, info, uctx);
gSignalThread = nullptr; gSignalThread = kInvalidThreadId;
// Kill ourselves with the previous handler. // Kill ourselves with the previous handler.
callPreviousSignalHandler(signum); callPreviousSignalHandler(signum);
} }
......
...@@ -45,6 +45,7 @@ TEST(SignalHandler, Simple) { ...@@ -45,6 +45,7 @@ TEST(SignalHandler, Simple) {
addFatalSignalCallback(callback1); addFatalSignalCallback(callback1);
addFatalSignalCallback(callback2); addFatalSignalCallback(callback2);
installFatalSignalHandler(); installFatalSignalHandler();
installFatalSignalCallbacks();
EXPECT_DEATH( EXPECT_DEATH(
failHard(), failHard(),
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment