Refactor android stacktrace
Move global state into a singleton class. Eliminates use of
GlobalMutex. Also use std::atomic rather than volatile, for improved
thread safety.
Bug: webrtc:11567
Change-Id: I305d16ed2f4a9a6497df119e66d9bba08337339a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/258120
Reviewed-by: Magnus Jedvert <magjed@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36475}
diff --git a/sdk/android/native_api/stacktrace/stacktrace.cc b/sdk/android/native_api/stacktrace/stacktrace.cc
index 4889437..96e03e0 100644
--- a/sdk/android/native_api/stacktrace/stacktrace.cc
+++ b/sdk/android/native_api/stacktrace/stacktrace.cc
@@ -83,6 +83,12 @@
// Struct to store the arguments to the signal handler.
struct SignalHandlerOutputState {
+ // This function is called iteratively for each stack trace element and stores
+ // the element in the array from `unwind_output_state`.
+ static _Unwind_Reason_Code UnwindBacktrace(
+ struct _Unwind_Context* unwind_context,
+ void* unwind_output_state);
+
// This event is signalled when signal handler is done executing.
AsyncSafeWaitableEvent signal_handler_finish_event;
// Running counter of array index below.
@@ -91,17 +97,11 @@
uintptr_t addresses[kMaxStackSize];
};
-// Global lock to ensure only one thread gets interrupted at a time.
-ABSL_CONST_INIT GlobalMutex g_signal_handler_lock(absl::kConstInit);
-// Argument passed to the ThreadSignalHandler() from the sampling thread to the
-// sampled (stopped) thread. This value is set just before sending signal to the
-// thread and reset when handler is done.
-SignalHandlerOutputState* volatile g_signal_handler_output_state;
-
// This function is called iteratively for each stack trace element and stores
// the element in the array from `unwind_output_state`.
-_Unwind_Reason_Code UnwindBacktrace(struct _Unwind_Context* unwind_context,
- void* unwind_output_state) {
+_Unwind_Reason_Code SignalHandlerOutputState::UnwindBacktrace(
+ struct _Unwind_Context* unwind_context,
+ void* unwind_output_state) {
SignalHandlerOutputState* const output_state =
static_cast<SignalHandlerOutputState*>(unwind_output_state);
@@ -123,13 +123,43 @@
return _URC_NO_REASON;
}
+class GlobalStackUnwinder {
+ public:
+ static GlobalStackUnwinder& Get() {
+ static GlobalStackUnwinder* const instance = new GlobalStackUnwinder();
+ return *instance;
+ }
+ const char* CaptureRawStacktrace(int pid,
+ int tid,
+ SignalHandlerOutputState* params);
+
+ private:
+ GlobalStackUnwinder() { current_output_state_.store(nullptr); }
+
+ // Temporarily installed signal handler.
+ static void SignalHandler(int signum, siginfo_t* info, void* ptr);
+
+ Mutex mutex_;
+
+ // Accessed by signal handler.
+ static std::atomic<SignalHandlerOutputState*> current_output_state_;
+ // A signal handler mustn't use locks.
+ static_assert(std::atomic<SignalHandlerOutputState*>::is_always_lock_free);
+};
+
+std::atomic<SignalHandlerOutputState*>
+ GlobalStackUnwinder::current_output_state_;
+
// This signal handler is exectued on the interrupted thread.
-void SignalHandler(int signum, siginfo_t* info, void* ptr) {
+void GlobalStackUnwinder::SignalHandler(int signum,
+ siginfo_t* info,
+ void* ptr) {
// This should have been set by the thread requesting the stack trace.
SignalHandlerOutputState* signal_handler_output_state =
- g_signal_handler_output_state;
+ current_output_state_.load();
if (signal_handler_output_state != nullptr) {
- _Unwind_Backtrace(&UnwindBacktrace, signal_handler_output_state);
+ _Unwind_Backtrace(&SignalHandlerOutputState::UnwindBacktrace,
+ signal_handler_output_state);
signal_handler_output_state->signal_handler_finish_event.Signal();
}
}
@@ -138,9 +168,10 @@
// trace and interrupt the given tid. This function will block until the output
// thread stack trace has been stored in `params`. The return value is an error
// string on failure and null on success.
-const char* CaptureRawStacktrace(int pid,
- int tid,
- SignalHandlerOutputState* params) {
+const char* GlobalStackUnwinder::CaptureRawStacktrace(
+ int pid,
+ int tid,
+ SignalHandlerOutputState* params) {
// This function is under a global lock since we are changing the signal
// handler and using global state for the output. The lock is to ensure only
// one thread at a time gets captured. The lock also means we need to be very
@@ -153,8 +184,8 @@
act.sa_flags = SA_RESTART | SA_SIGINFO;
sigemptyset(&act.sa_mask);
- GlobalMutexLock ls(&g_signal_handler_lock);
- g_signal_handler_output_state = params;
+ MutexLock loch(&mutex_);
+ current_output_state_.store(params);
if (sigaction(kSignal, &act, &old_act) != 0)
return "Failed to change signal action";
@@ -210,7 +241,8 @@
// `g_signal_handler_param`.
SignalHandlerOutputState params;
- const char* error_string = CaptureRawStacktrace(getpid(), tid, ¶ms);
+ const char* error_string =
+ GlobalStackUnwinder::Get().CaptureRawStacktrace(getpid(), tid, ¶ms);
if (error_string != nullptr) {
RTC_LOG(LS_ERROR) << error_string << ". tid: " << tid
<< ". errno: " << errno;
@@ -224,7 +256,7 @@
std::vector<StackTraceElement> GetStackTrace() {
SignalHandlerOutputState params;
- _Unwind_Backtrace(&UnwindBacktrace, ¶ms);
+ _Unwind_Backtrace(&SignalHandlerOutputState::UnwindBacktrace, ¶ms);
if (params.stack_size_counter >= kMaxStackSize) {
RTC_LOG(LS_WARNING) << "Stack trace was truncated";
}