fix(libxce): break ABBA iomgr deadlock — make endpoint busy flag lockless (bug-1282)

Root-caused via gdb (all-thread backtrace on two independently-wedged controllers
during chaotic failover): an ABBA lock-order deadlock in the async dispatch path.

  io_worker (netio.c): holds ep->lock, blocks on q->lock (vtask_enq -> dataq_enq)
  vdispatcher: dataq_deq holds q->lock, blocks on ep->lock via the
    vtask_eligible -> endpoint_is_task_blocked match callback

endpoint_is_task_blocked took ep->lock while the caller (dataq_deq) held the dataq
q->lock, inverting io_worker's ep->lock -> q->lock order. Under churn the two
orders collide and the iomgr wedges (no more accepts / vote replies).

Fix (mirrors the dispatch_inflight precedent, bug-1197): promote endpoint_t.busy
from a 1-bit bitfield to _Atomic int and make endpoint_block_tasks /
endpoint_unblock_tasks / endpoint_is_task_blocked lockless (release/acquire). The
ep->lock that wrapped each access protected nothing else (busy only gates the
vtask queue), so removing it is safe; the inherent set-then-check TOCTOU is
unchanged (the lock never prevented it — the decision is advisory).

Verified: post-fix gdb soak shows ZERO threads in __lll_lock_wait and
endpoint_is_task_blocked no longer appears in any lock path (deadlock gone);
controller_unit_test all green (no regression). NOTE: a SEPARATE release-only
restart segfault remains and still blocks the raft-ha-core-safety 7.2 gate
(tracked separately) — deadlocks hang, they do not SIGSEGV, so it is a distinct
memory bug.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019dbTHR9CsEYK9RsqmQpQpC
This commit is contained in:
Rongsong Shen 2026-06-30 08:22:15 +00:00
parent bfe69b1e85
commit 25e741d8c7
2 changed files with 20 additions and 12 deletions

View file

@ -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

View file

@ -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");