diff --git a/src/include/netio.h b/src/include/netio.h index 1118747..ee0dbc9 100644 --- a/src/include/netio.h +++ b/src/include/netio.h @@ -72,7 +72,14 @@ typedef struct endpoint_t { int fd; int idx; int flags; - unsigned int busy : 1; + /* bug-1282: `busy` is read by endpoint_is_task_blocked from the vtask- + * eligibility match callback (vtask_eligible), which runs UNDER the dataq + * q->lock. Taking ep->lock there inverted io_worker's ep->lock -> q->lock + * order (io_worker holds ep->lock and enqueues via dataq_enq), an ABBA + * deadlock that wedged the iomgr under chaotic failover churn. Promoted from + * a bitfield to a full _Atomic int (bitfields cannot be _Atomic — same reason + * as dispatch_inflight) so the eligibility check reads it locklessly. */ + _Atomic int busy; unsigned int tearing_down : 1; unsigned int ssl_handshake_done : 1; /* bug-1197: count of peer-control dispatch attempts currently between the diff --git a/src/lib/netio.c b/src/lib/netio.c index c756ca0..54aa1f1 100644 --- a/src/lib/netio.c +++ b/src/lib/netio.c @@ -981,30 +981,31 @@ uint64_t endpoint_getseq(endpoint_t* ep) return seq; } +/* bug-1282: `busy` is a self-contained eligibility flag — the ep->lock that used + * to wrap each access protected nothing else (it only gates the vtask queue). + * Reading it under ep->lock from the vtask-eligibility path + * (endpoint_is_task_blocked, called inside dataq_deq while holding the dataq + * q->lock) inverted io_worker's ep->lock -> q->lock order and deadlocked the + * iomgr (ABBA). `busy` is now an _Atomic int, so all three sites use lockless + * release/acquire and never touch ep->lock. The inherent set-then-check TOCTOU + * (busy may change right after the read) is unchanged — the lock never prevented + * it; the eligibility decision was always advisory. */ void endpoint_block_tasks(endpoint_t* ep) { TRACE("%s[%ld] block tasks for endpoint %p", __func__, pthread_self(), ep); - mylock_lock(&(ep->lock)); - ep->busy = 1; - mylock_unlock(&(ep->lock)); + atomic_store_explicit(&ep->busy, 1, memory_order_release); } void endpoint_unblock_tasks(endpoint_t* ep) { TRACE("%s[%ld] unblock tasks for endpoint %p", __func__, pthread_self(), ep); - mylock_lock(&(ep->lock)); - ep->busy = 0; - mylock_unlock(&(ep->lock)); + atomic_store_explicit(&ep->busy, 0, memory_order_release); } int endpoint_is_task_blocked(endpoint_t* ep) { - int busy; - - mylock_lock(&(ep->lock)); - busy = ep->busy; - mylock_unlock(&(ep->lock)); + int busy = atomic_load_explicit(&ep->busy, memory_order_acquire); TRACE("%s[%ld] endpoint %p task %s", __func__, pthread_self(), ep, (busy != 0) ? "blocked" : "not blocked");