This is an automated email from the ASF dual-hosted git repository.

guangmingchen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brpc.git


The following commit(s) were added to refs/heads/master by this push:
     new b6ae7bf9 Bugfix: Signal Trace mode may send SIGURG to wrong thread 
(#3039)
b6ae7bf9 is described below

commit b6ae7bf97314c1234883060062abd80a3af44be4
Author: chenyujie <78433231+codez...@users.noreply.github.com>
AuthorDate: Tue Aug 26 14:16:52 2025 +0800

    Bugfix: Signal Trace mode may send SIGURG to wrong thread (#3039)
    
    * Bugfix: Use `pthread_sigqueue` to send the signal to correct worker 
thread'
    
    * Change the type of `TaskGroup::_tid` to `pthread_t`
---
 src/bthread/task_control.cpp |  3 ++-
 src/bthread/task_group.cpp   |  2 +-
 src/bthread/task_group.h     |  4 ++--
 src/bthread/task_meta.h      |  2 +-
 src/bthread/task_tracer.cpp  | 35 ++++++++++++++++++++---------------
 src/bthread/task_tracer.h    | 20 ++++++++++----------
 6 files changed, 36 insertions(+), 30 deletions(-)

diff --git a/src/bthread/task_control.cpp b/src/bthread/task_control.cpp
index 05ceec09..1bdcf0c0 100644
--- a/src/bthread/task_control.cpp
+++ b/src/bthread/task_control.cpp
@@ -19,6 +19,7 @@
 
 // Date: Tue Jul 10 17:40:58 CST 2012
 
+#include <pthread.h>
 #include <sys/syscall.h>                   // SYS_gettid
 #include "butil/scoped_lock.h"             // BAIDU_SCOPED_LOCK
 #include "butil/errno.h"                   // berror
@@ -91,7 +92,7 @@ void* TaskControl::worker_thread(void* arg) {
         return NULL;
     }
 
-    g->_tid = syscall(SYS_gettid);
+    g->_tid = pthread_self();
 
     if (FLAGS_task_group_set_worker_name) {
         std::string worker_thread_name = butil::string_printf(
diff --git a/src/bthread/task_group.cpp b/src/bthread/task_group.cpp
index d4cb81f6..773a442b 100644
--- a/src/bthread/task_group.cpp
+++ b/src/bthread/task_group.cpp
@@ -1107,7 +1107,7 @@ void print_task(std::ostream& os, bthread_t tid) {
     TaskStatistics stat = {0, 0, 0};
     TaskStatus status = TASK_STATUS_UNKNOWN;
     bool traced = false;
-    pid_t worker_tid = 0;
+    pthread_t worker_tid{};
     {
         BAIDU_SCOPED_LOCK(m->version_lock);
         if (given_ver == *m->version_butex) {
diff --git a/src/bthread/task_group.h b/src/bthread/task_group.h
index b79e69d8..43c91152 100644
--- a/src/bthread/task_group.h
+++ b/src/bthread/task_group.h
@@ -219,7 +219,7 @@ public:
 
     bthread_tag_t tag() const { return _tag; }
 
-    pid_t tid() const { return _tid; }
+    pthread_t tid() const { return _tid; }
 
     int64_t current_task_cpu_clock_ns() {
         if (_last_cpu_clock_ns == 0) {
@@ -378,7 +378,7 @@ friend class TaskControl;
     bthread_tag_t _tag{BTHREAD_TAG_DEFAULT};
 
     // Worker thread id.
-    pid_t _tid{-1};
+    pthread_t _tid{};
 };
 
 }  // namespace bthread
diff --git a/src/bthread/task_meta.h b/src/bthread/task_meta.h
index 34d86632..a4ed42bf 100644
--- a/src/bthread/task_meta.h
+++ b/src/bthread/task_meta.h
@@ -113,7 +113,7 @@ struct TaskMeta {
     // Whether bthread is traced?
     bool traced{false};
     // Worker thread id.
-    pid_t worker_tid{-1};
+    pthread_t worker_tid{};
 
 public:
     // Only initialize [Not Reset] fields, other fields will be reset in
diff --git a/src/bthread/task_tracer.cpp b/src/bthread/task_tracer.cpp
index 32d103cd..e2483495 100644
--- a/src/bthread/task_tracer.cpp
+++ b/src/bthread/task_tracer.cpp
@@ -18,17 +18,18 @@
 #ifdef BRPC_BTHREAD_TRACER
 
 #include "bthread/task_tracer.h"
-#include <unistd.h>
-#include <poll.h>
-#include <gflags/gflags.h>
-#include <absl/debugging/stacktrace.h>
-#include <absl/debugging/symbolize.h>
-#include "butil/debug/stack_trace.h"
+#include "bthread/processor.h"
+#include "bthread/task_group.h"
+#include "butil/fd_utility.h"
 #include "butil/memory/scope_guard.h"
 #include "butil/reloadable_flags.h"
-#include "butil/fd_utility.h"
-#include "bthread/task_group.h"
-#include "bthread/processor.h"
+#include <absl/debugging/stacktrace.h>
+#include <absl/debugging/symbolize.h>
+#include <csignal>
+#include <gflags/gflags.h>
+#include <poll.h>
+#include <pthread.h>
+#include <unistd.h>
 
 namespace bthread {
 
@@ -151,7 +152,7 @@ void TaskTracer::set_status(TaskStatus s, TaskMeta* m) {
             m->status = s;
         }
         if (TASK_STATUS_CREATED == s) {
-            m->worker_tid = -1;
+            m->worker_tid = pthread_t{};
         }
     }
 
@@ -161,7 +162,7 @@ void TaskTracer::set_status(TaskStatus s, TaskMeta* m) {
     }
 }
 
-void TaskTracer::set_running_status(pid_t worker_tid, TaskMeta* m) {
+void TaskTracer::set_running_status(pthread_t worker_tid, TaskMeta* m) {
     BAIDU_SCOPED_LOCK(m->version_lock);
     m->worker_tid = worker_tid;
     m->status = TASK_STATUS_RUNNING;
@@ -230,7 +231,7 @@ TaskTracer::Result TaskTracer::TraceImpl(bthread_t tid) {
 
     BAIDU_SCOPED_LOCK(_mutex);
     TaskStatus status;
-    pid_t worker_tid;
+    pthread_t worker_tid;
     const uint32_t given_version = get_version(tid);
     {
         BAIDU_SCOPED_LOCK(m->version_lock);
@@ -368,7 +369,7 @@ void TaskTracer::SignalHandler(int, siginfo_t* info, void* 
context) {
     butil::ignore_result(write(signal_sync->pipe_fds[1], "1", 1));
 }
 
-TaskTracer::Result TaskTracer::SignalTrace(pid_t tid) {
+TaskTracer::Result TaskTracer::SignalTrace(pthread_t worker_tid) {
     // CAUTION:
     // 
https://github.com/gperftools/gperftools/wiki/gperftools'-stacktrace-capturing-methods-and-their-issues#libunwind
     // Generally, libunwind promises async-signal-safety for capturing 
backtraces.
@@ -403,6 +404,10 @@ TaskTracer::Result TaskTracer::SignalTrace(pid_t tid) {
     //
     // Therefore, use async-signal-safe absl::DefaultStackUnwinder instead of 
libunwind.
 
+    if (pthread_equal(worker_tid, pthread_self())) {
+        return Result::MakeErrorResult("Forbid to trace self");
+    }
+
     // Remove unused SignalSyncs.
     auto iter = std::remove_if(
         _inuse_signal_syncs.begin(), _inuse_signal_syncs.end(),
@@ -424,11 +429,11 @@ TaskTracer::Result TaskTracer::SignalTrace(pid_t tid) {
     sigval value{};
     value.sival_ptr = signal_sync.get();
     size_t sigqueue_try = 0;
-    while (sigqueue(tid, SIGURG, value) != 0) {
+    while (pthread_sigqueue(worker_tid, SIGURG, value) != 0) {
         if (errno != EAGAIN || sigqueue_try++ >= 3) {
             // Remove reference for SignalHandler.
             signal_sync->RemoveRefManually();
-            return Result::MakeErrorResult("Fail to sigqueue: %s, tid: %d", 
berror(), tid);
+            return Result::MakeErrorResult("Fail to pthread_sigqueue: %s", 
berror());
         }
     }
     _inuse_signal_syncs.push_back(signal_sync);
diff --git a/src/bthread/task_tracer.h b/src/bthread/task_tracer.h
index 81940cfd..0888c658 100644
--- a/src/bthread/task_tracer.h
+++ b/src/bthread/task_tracer.h
@@ -20,15 +20,15 @@
 
 #ifdef BRPC_BTHREAD_TRACER
 
-#include <signal.h>
-#include <vector>
-#include <libunwind.h>
-#include "butil/synchronization/condition_variable.h"
+#include "bthread/mutex.h"
+#include "bthread/task_meta.h"
 #include "butil/intrusive_ptr.hpp"
-#include "butil/strings/safe_sprintf.h"
 #include "butil/shared_object.h"
-#include "bthread/task_meta.h"
-#include "bthread/mutex.h"
+#include "butil/strings/safe_sprintf.h"
+#include "butil/synchronization/condition_variable.h"
+#include <libunwind.h>
+#include <signal.h>
+#include <vector>
 
 namespace bthread {
 
@@ -39,10 +39,10 @@ public:
     bool Init();
     // Set the status to `s'.
     void set_status(TaskStatus s, TaskMeta* meta);
-    static void set_running_status(pid_t worker_tid, TaskMeta* meta);
+    static void set_running_status(pthread_t worker_tid, TaskMeta* meta);
     static bool set_end_status_unsafe(TaskMeta* m);
 
-    // Trace the bthread of `tid'.
+    // Trace the bthread of `tid`.
     std::string Trace(bthread_t tid);
     void Trace(std::ostream& os, bthread_t tid);
 
@@ -103,7 +103,7 @@ private:
 
     static bool RegisterSignalHandler();
     static void SignalHandler(int sig, siginfo_t* info, void* context);
-    Result SignalTrace(pid_t worker_tid);
+    Result SignalTrace(pthread_t worker_tid);
 
     // Make sure only one bthread is traced at a time.
     Mutex _trace_request_mutex;


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org

Reply via email to