Skip to content

epoll: hijackConn mutates worker-thread-only liveConns and connCount from the dispatch goroutine (the epoll twin of celeris#539) #668

Description

@FumingPower3925

Found while fixing celeris#654 (PR #665). Not reproduced — this is a reading, with the exact lines at df1269c.

What runs off-thread

With AsyncHandlers: true the user handler runs inside ProcessH1 on the dispatch goroutine, so Context.Hijack → hijackConn (engine/epoll/loop.go:1466) performs the whole engine-side teardown on that goroutine. The lines it executes do not all stand on the same ground:

liveConns is documented Worker-thread-only — no synchronization. (#318) (loop.go:86) and connCount is a bare int (loop.go:80).

Two concrete consequences

  1. Torn sweep. checkTimeouts (:2575, loop at :2577) and shutdown's phase 1 read len(l.liveConns) once and then index l.liveConns[i] each iteration. removeLiveConn swaps-with-last and shrinks the slice, so a hijack concurrent with a sweep can index out of range, or move an unvisited conn into an already-visited slot and skip it for that pass.
  2. Lost decrement. l.connCount-- here races acceptAll's l.connCount++ (:914). A lost update leaves connCount permanently wrong, and l.connCount == 0 (:742) is the gate that lets a draining loop reach SUSPENDED — so a loop that loses one increment never suspends again.

Why this is separate from celeris#654

#654 / #665 removes the duplicate teardown: a loop-thread closeConn that parked on cs.detachMu and then redid work hijackConn had already done. It does not change where hijackConn's own teardown runs. #665's regression test observes correct counters only because its asyncClosed barrier orders the two goroutines — that ordering belongs to the test, not to production. The comment in hijackConn that used to assert these fields "are only read by the worker between epoll_wait returns, never while a dispatch goroutine is mid-ProcessH1" is exactly the premise #654 disproves; #665 replaces it with a per-line statement of what is and is not serialized, and points here.

io_uring does not carry this: its hijackConn refuses when w.async (celeris#539). The price of that refusal is celeris#558.

Fix directions

How to judge a fix

go test -race ./engine/epoll/ with a test that hijacks from a dispatch goroutine while a sweep walks liveConns: the race detector should report the liveConns / connCount accesses today and be silent after. Counter-level witness: accept and hijack concurrently in a loop, then assert connCount returns to 0 and the loop can still reach SUSPENDED.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/engineEngine interface or implementationbugSomething isn't workingengine/epollEpoll engine specificsplatform/linuxLinux-specific (io_uring, epoll)

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions