Skip to content

epoll: a driver conn's read loop reads the caller's descriptor number after UnregisterConn has returned, and drops the bytes of the file that took the number #710

Description

@FumingPower3925

Found in the review of #696 (celeris#691). Pre-existing on main (9f4d89b); #696 does not touch engine/epoll, and #674 (1d90b5d) does not change engine/epoll/driver.go.

The defect

The epoll worker reads a driver conn by the caller's descriptor number, and checks whether the conn was unregistered only after the read:

  • driverRead (engine/epoll/driver.go:316) loops unix.Read(dc.fd, ...) at :321, and checks dc.closed under dc.mu at :334, after the bytes are read. The loop reads again after onRecv whenever the previous read filled the 32 KiB buffer.
  • UnregisterConn (:151) runs on the caller's goroutine. It deletes the conn, runs EPOLL_CTL_DEL and fires onClose, and returns. Nothing fences it against a worker that is already inside that conn's read loop, or that looked the conn up (engine/epoll/loop.go:572) before the delete.

So a caller that unregisters while the worker is in the conn's read loop, then closes fd, and whose number is taken by another file X before the worker's next read, has the worker read X's bytes. The worker then sees dc.closed and drops them. If X is a blocking descriptor with nothing to read, the read blocks the worker.

Measured

A probe (zz_probe691r3_epollread_test.go, not committed): A's peer holds more than one read buffer before A is registered, so the worker's first read fills the buffer. A's first onRecv parks the worker. The caller calls UnregisterConn(A), which returns with onClose fired, closes A's fd, and a new socket X takes the number. X's peer writes 10 bytes. Then the worker is released.

head runs A's first read X took A's number X's reader 300 ms after release
443b629 (#696 round 3; engine/epoll = 9f4d89b) 3 32768 bytes, a full buffer yes, as the lowest free number, 3/3 EAGAIN, 3/3: the 10 bytes were consumed and dropped

Laptop Docker, golang:1.27, -race, kernel 7.0.12-linuxkit. Evidence (maintainer's evidence tree): evidence/celeris-691/lane-20260926/round3/, probe patches/zz_probe691r3_epollread_test.go, script tools/s4-epollprobe.sh, log logs/s4-01-epollprobe-m8.log.

Who reaches it

A driver on the epoll engine whose conn has more than 32 KiB queued when it is unregistered from another goroutine, and whose process opens a descriptor right after the close. In-tree drivers unregister from their own goroutines (redis Pub/Sub Close, for one) and close at once.

The engine.WorkerLoop contract text #696 adds promises that the caller may close fd as soon as UnregisterConn returns. #696 scopes that promise to io_uring and points here for epoll.

Fix direction

Either one closes it:

  1. Read and write through an engine-owned duplicate of fd, taken at RegisterConn, as the io_uring engine does since fix(iouring): run every driver op through the engine's own duplicate of the socket, and count every cancel until its CQE, so closing after UnregisterConn is safe (celeris#691, celeris#707) #696. A read after the unregister then reaches the unregistered socket, never the file that took the number. EPOLL_CTL_DEL still has to name the registered file.
  2. Make UnregisterConn wait until the worker is outside the conn's read loop, for example with a per-conn in-use flag the worker holds across the read loop. UnregisterConn runs on the caller's goroutine, so it must not wait when it is called from that conn's own onRecv.

The probe above is the failing-first test for either.

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

    bugSomething isn't workingengine/epollEpoll engine specifics

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions