Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/bthread/task_control.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -343,8 +343,7 @@ void TaskControl::stop_and_join() {
for (auto worker: _workers) {
// Interrupt blocking operations.
#ifdef BRPC_BTHREAD_TRACER
// TaskTracer has registered signal handler for SIGURG.
pthread_kill(worker, SIGURG);
pthread_kill(worker, _task_tracer.get_trace_signal());
#else
interrupt_pthread(worker);
#endif // BRPC_BTHREAD_TRACER
Expand Down
12 changes: 9 additions & 3 deletions src/bthread/task_tracer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ namespace bthread {

DEFINE_uint32(signal_trace_timeout_ms, 50, "Timeout for signal trace in ms");
BUTIL_VALIDATE_GFLAG(signal_trace_timeout_ms, butil::PositiveInteger<uint32_t>);
// Note that SIGURG handler may be registered by some library such as cgo
// so let the signal number be configurable
DEFINE_int32(signal_number_for_trace, SIGURG,
"signal number used for stack trace, default to SIGURG");
Comment thread
chenBright marked this conversation as resolved.

extern BAIDU_THREAD_LOCAL TaskMeta* pthread_fake_meta;

Expand Down Expand Up @@ -315,17 +319,19 @@ TaskTracer::Result TaskTracer::TraceByLibunwind(unw_cursor_t& cursor) {

bool TaskTracer::RegisterSignalHandler() {
// Set up the signal handler.
_signal_num = FLAGS_signal_number_for_trace;
struct sigaction old_sa{};
struct sigaction sa{};
sa.sa_sigaction = SignalHandler;
sa.sa_flags = SA_SIGINFO;
sigfillset(&sa.sa_mask);
if (sigaction(SIGURG, &sa, &old_sa) != 0) {
if (sigaction(_signal_num, &sa, &old_sa) != 0) {
PLOG(ERROR) << "Failed to sigaction";
return false;
}
if (NULL != old_sa.sa_handler || NULL != old_sa.sa_sigaction) {
LOG(ERROR) << "Signal handler of SIGURG is already registered";
LOG(ERROR) << "Signal handler of signal number "
<< _signal_num << " is already registered";
return false;
}

Expand Down Expand Up @@ -403,7 +409,7 @@ TaskTracer::Result TaskTracer::SignalTrace(pthread_t worker_tid) {
sigval value{};
value.sival_ptr = signal_sync.get();
size_t sigqueue_try = 0;
while (pthread_sigqueue(worker_tid, SIGURG, value) != 0) {
while (pthread_sigqueue(worker_tid, _signal_num, value) != 0) {
if (errno != EAGAIN || sigqueue_try++ >= 3) {
// Remove reference for SignalHandler.
signal_sync->RemoveRefManually();
Expand Down
6 changes: 5 additions & 1 deletion src/bthread/task_tracer.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ class TaskTracer {
// When the worker is jumping stack from a bthread to another,
static void WaitForTracing(TaskMeta* m);

int get_trace_signal() const {
return _signal_num;
}
private:
// Error number guard used in signal handler.
class ErrnoGuard {
Expand Down Expand Up @@ -100,7 +103,7 @@ class TaskTracer {
Result ContextTrace(bthread_fcontext_t fcontext);
static Result TraceByLibunwind(unw_cursor_t& cursor);

Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method signature changed from static to non-static, which is a significant API change affecting how this method is called. This change should be documented with a comment explaining why it needs to be non-static (i.e., to access the instance variable _signal_num).

Suggested change
// This method must be non-static in order to access the instance variable _signal_num.

Copilot uses AI. Check for mistakes.
static bool RegisterSignalHandler();
bool RegisterSignalHandler();
static void SignalHandler(int sig, siginfo_t* info, void* context);
Result SignalTrace(pthread_t worker_tid);

Expand All @@ -115,6 +118,7 @@ class TaskTracer {
std::vector<butil::intrusive_ptr<SignalSync>> _inuse_signal_syncs;

bvar::LatencyRecorder _trace_time{"bthread_trace_time"};
int _signal_num{SIGURG};
Comment thread
chenBright marked this conversation as resolved.
};

} // namespace bthread
Expand Down
10 changes: 10 additions & 0 deletions test/brpc_builtin_service_unittest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,19 @@ DECLARE_bool(rpcz_hex_log_id);
DECLARE_int32(idle_timeout_second);
} // namespace rpc

#ifdef BRPC_BTHREAD_TRACER
namespace bthread {
DECLARE_int32(signal_number_for_trace);
}
#endif

int main(int argc, char* argv[]) {
brpc::FLAGS_idle_timeout_second = 0;
testing::InitGoogleTest(&argc, argv);
#ifdef BRPC_BTHREAD_TRACER
// test using signal number other than SIGURG
bthread::FLAGS_signal_number_for_trace = SIGUSR2;
#endif
return RUN_ALL_TESTS();
}

Expand Down
Loading