Bugfix: SignalSync leaks and wrong status of global priority bthread

Author: chenBright

src/bthread/task_control.cpp

@@ -209,7 +209,7 @@ int TaskControl::init(int concurrency) {
             "bthread_worker_usage", tag_str, _tagged_cumulated_worker_time[i], 1));
         _tagged_nbthreads.push_back(new bvar::Adder<int64_t>("bthread_count", tag_str));
         if (_priority_queues[i].init(BTHREAD_MAX_CONCURRENCY) != 0) {
-            LOG(FATAL) << "Fail to init _priority_q";
+            LOG(ERROR) << "Fail to init _priority_q";
             return -1;
         }
     }

src/bthread/task_group.cpp

@@ -496,7 +496,7 @@ int TaskGroup::start_foreground(TaskGroup** pg,
             (bool)(using_attr.flags & BTHREAD_NOSIGNAL)
         };
         g->set_remained(fn, &args);
-        TaskGroup::sched_to(pg, m->tid);
+        sched_to(pg, m->tid);
     }
     return 0;
 }
@@ -784,6 +784,11 @@ void TaskGroup::ready_to_run(TaskMeta* meta, bool nosignal) {
 #ifdef BRPC_BTHREAD_TRACER
     _control->_task_tracer.set_status(TASK_STATUS_READY, meta);
 #endif // BRPC_BTHREAD_TRACER
+    if (meta->attr.flags & BTHREAD_GLOBAL_PRIORITY) {
+        _control->push_priority_queue(_tag, meta->tid);
+        return;
+    }
+
     push_rq(meta->tid);
     if (nosignal) {
         ++_num_nosignal;
@@ -808,6 +813,11 @@ void TaskGroup::ready_to_run_remote(TaskMeta* meta, bool nosignal) {
 #ifdef BRPC_BTHREAD_TRACER
     _control->_task_tracer.set_status(TASK_STATUS_READY, meta);
 #endif // BRPC_BTHREAD_TRACER
+    if (meta->attr.flags & BTHREAD_GLOBAL_PRIORITY) {
+        _control->push_priority_queue(_tag, meta->tid);
+        return;
+    }
+
     _remote_rq._mutex.lock();
     while (!_remote_rq.push_locked(meta->tid)) {
         flush_nosignal_tasks_remote_locked(_remote_rq._mutex);
@@ -870,6 +880,10 @@ void TaskGroup::ready_to_run_in_worker_ignoresignal(void* args_in) {
 
 void TaskGroup::priority_to_run(void* args_in) {
     ReadyToRunArgs* args = static_cast<ReadyToRunArgs*>(args_in);
+#ifdef BRPC_BTHREAD_TRACER
+    tls_task_group->_control->_task_tracer.set_status(
+        TASK_STATUS_READY, args->meta);
+#endif // BRPC_BTHREAD_TRACER
     return tls_task_group->control()->push_priority_queue(args->tag, args->meta->tid);
 }
 
@@ -1031,14 +1045,15 @@ int TaskGroup::interrupt(bthread_t tid, TaskControl* c, bthread_tag_t tag) {
         }
     } else if (sleep_id != 0) {
         if (get_global_timer_thread()->unschedule(sleep_id) == 0) {
-            bthread::TaskGroup* g = bthread::tls_task_group;
+            TaskGroup* g = tls_task_group;
+            TaskMeta* m = address_meta(tid);
             if (g) {
-                g->ready_to_run(TaskGroup::address_meta(tid));
+                g->ready_to_run(m);
             } else {
                 if (!c) {
                     return EINVAL;
                 }
-                c->choose_one_group(tag)->ready_to_run_remote(TaskGroup::address_meta(tid));
+                c->choose_one_group(tag)->ready_to_run_remote(m);
             }
         }
     }

src/bthread/task_tracer.cpp

@@ -353,8 +353,9 @@ bool TaskTracer::RegisterSignalHandler() {
 // Caution: This function should be async-signal-safety.
 void TaskTracer::SignalHandler(int, siginfo_t* info, void* context) {
     ErrnoGuard guard;
+    // Ref has been taken before the signal is sent, so no need to add ref here.
     butil::intrusive_ptr<SignalSync> signal_sync(
-        static_cast<SignalSync*>(info->si_value.sival_ptr));
+        static_cast<SignalSync*>(info->si_value.sival_ptr), false);
     if (NULL == signal_sync || NULL == signal_sync->result) {
         // The signal is not from Tracer, such as TaskControl, do nothing.
         return;
@@ -408,7 +409,7 @@ TaskTracer::Result TaskTracer::SignalTrace(pid_t tid) {
     auto iter = std::remove_if(
         _inuse_signal_syncs.begin(), _inuse_signal_syncs.end(),
         [](butil::intrusive_ptr<SignalSync>& sync) {
-            return sync->ref_count() == 0;
+            return sync->ref_count() == 1;
     });
     _inuse_signal_syncs.erase(iter, _inuse_signal_syncs.end());