Skip to content

Commit

Permalink
KUDU-3093: another band-aid for this DebugUtilTest.TestSignalStackTrace
Browse files Browse the repository at this point in the history
A TSAN build yielded a stack trace like:

@           0x444a88  __tsan::ProcessPendingSignals()
@           0x4541c1  __interceptor_pthread_mutex_trylock
@     0x7fbca26124e1  kudu::Mutex::TryAcquire()
@     0x7fbca2612893  kudu::Mutex::Acquire()
@           0x4fe036  kudu::MutexLock::MutexLock()
@           0x504abd  kudu::CountDownLatch::WaitUntil()
@           0x504a5f  kudu::CountDownLatch::WaitFor()
@           0x4f04a9  kudu::(anonymous namespace)::SleeperThread()
...

Rather than find the synchronization primitive frame least likely to be
inlined, let's take a more comprehensive approach and search for multiple
candidate frames, including SleeperThread.

I tested this locally in DEBUG, RELEASE, ASAN, and TSAN modes.

Change-Id: Ia4ca0f48ba1d7ad4cea40b70af271d7948f78a57
Reviewed-on: http://gerrit.cloudera.org:8080/15605
Reviewed-by: Alexey Serbin <[email protected]>
Tested-by: Kudu Jenkins
  • Loading branch information
adembo committed Mar 31, 2020
1 parent ce82af1 commit 263c3aa
Showing 1 changed file with 12 additions and 5 deletions.
17 changes: 12 additions & 5 deletions src/kudu/util/debug-util-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include <gtest/gtest.h>

#include "kudu/gutil/ref_counted.h"
#include "kudu/gutil/strings/substitute.h"
#include "kudu/gutil/walltime.h"
#include "kudu/util/array_view.h"
#include "kudu/util/countdown_latch.h"
Expand All @@ -53,6 +54,7 @@

using std::string;
using std::vector;
using strings::Substitute;

DECLARE_int32(test_timeout_after);
DECLARE_int32(stress_cpu_threads);
Expand Down Expand Up @@ -121,11 +123,16 @@ TEST_F(DebugUtilTest, TestSignalStackTrace) {
// to start up and actually call our function.
//
// Note: due to RELEASE build inlining, we need to make sure to pick a stack
// frame that isn't optimized away.
static constexpr const char* kTestThreadStackFrame =
// frame that isn't optimized away. Different compilers behave differently, so
// we'll use a regular expression to get maximal coverage.
static constexpr const char* kTestThreadOneStackFrame =
"SleeperThread";
static constexpr const char* kTestThreadAnotherStackFrame =
"kudu::ConditionVariable::WaitUntil()";
static const string kStackFrameRegExp = Substitute(
"$0|$1", kTestThreadOneStackFrame, kTestThreadAnotherStackFrame);
ASSERT_EVENTUALLY([&]() {
ASSERT_STR_CONTAINS(DumpThreadStack(t->tid()), kTestThreadStackFrame);
ASSERT_STR_MATCHES(DumpThreadStack(t->tid()), kStackFrameRegExp);
});

// Test that we can change the signal and that the stack traces still work,
Expand All @@ -140,15 +147,15 @@ TEST_F(DebugUtilTest, TestSignalStackTrace) {
ASSERT_FALSE(IsSignalHandlerRegistered(SIGUSR2));

// Stack traces should work using the new handler.
ASSERT_STR_CONTAINS(DumpThreadStack(t->tid()), kTestThreadStackFrame);
ASSERT_STR_MATCHES(DumpThreadStack(t->tid()), kStackFrameRegExp);

// Switch back to SIGUSR2 and ensure it changes back.
ASSERT_OK(SetStackTraceSignal(SIGUSR2));
ASSERT_TRUE(IsSignalHandlerRegistered(SIGUSR2));
ASSERT_FALSE(IsSignalHandlerRegistered(SIGHUP));

// Stack traces should work using the new handler.
ASSERT_STR_CONTAINS(DumpThreadStack(t->tid()), kTestThreadStackFrame);
ASSERT_STR_MATCHES(DumpThreadStack(t->tid()), kStackFrameRegExp);

// Register our own signal handler on SIGHUP, and ensure that
// we get a bad Status if we try to use it.
Expand Down

0 comments on commit 263c3aa

Please sign in to comment.