two-headBoy commented on code in PR #3633:
URL: https://github.com/apache/brpc/pull/3633#discussion_r4347149544
##########
src/bthread/task_group_inl.h:
##########
@@ -93,11 +93,65 @@ inline void TaskGroup::push_rq(bthread_t tid) {
// brpc)
flush_nosignal_tasks();
LOG_EVERY_SECOND(ERROR) << "_rq is full, capacity=" << _rq.capacity();
- // TODO(gejun): May cause deadlock when all workers are spinning here.
- // A better solution is to pop and run existing bthreads, however which
- // make set_remained()-callbacks do context switches and need extensive
- // reviews on related code.
- ::usleep(1000);
+ if (is_current_pthread_task() && !is_current_main_task()) {
+ // A pthread-mode bthread runs on _main_stack and cannot be
+ // scheduled out, fall back to waiting.
+ ::usleep(1000);
+ continue;
+ }
+ bthread_t run_tid = 0;
+#ifndef BTHREAD_FAIR_WSQ
+ const bool popped = _rq.pop(&run_tid);
+#else
+ const bool popped = _rq.steal(&run_tid);
+#endif
+ if (!popped) {
+ // The queue was drained by stealers between the failed push and
+ // the pop, retrying the push will succeed.
+ continue;
+ }
+ if (is_current_main_task() &&
+ address_meta(run_tid)->attr.stack_type ==
BTHREAD_STACKTYPE_PTHREAD) {
+ // pthread-mode tasks are run inline on _main_stack by the
+ // dispatch loops (run_main_task/task_runner). Switching to one
+ // from the main task itself causes no stack jump and would
+ // silently drop it, put it back and wait; stealers may run it
+ // on other workers.
+ _rq.push(run_tid);
Review Comment:
Fixed. The re-queue pushes now`CHECK` the return value. The push is
guaranteed to succeed here — the pops above freed at least that many
slots,`_rq` has a single producer (this worker) and stealers never push — but
the`CHECK` turns this invariant into a runtime assertion, so any future change
breaking it (e.g. WSQ semantics) fails loudly instead of silently dropping a
task.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
