forked from lei.chen/myxce
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:
parent
bfe69b1e85
commit
25e741d8c7
2 changed files with 20 additions and 12 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue