test(iouring): void a linger attempt whose close did not linger, and retry it (celeris#763) - #810
Conversation
…retry it (celeris#763) TestDriverLingeringCloseDoesNotStallTheWorker/linger failed in 3 of 30 linger runs on main's CI with "L's onClose fired while its close still lingered" when the close had not lingered at all: the kernel ends a close's linger wait early on a pending signal, the fixed engine then fires onClose at once, and the rig could not tell that from onClose firing before a lingering close returned. Each attempt now reads /proc/net/tcp, after V was served and after onClose was looked for, for a row that still carries the socket's inode: tcp_close orphans the socket only once its linger wait has ended. An attempt whose close has returned by then is void and is retried on a new engine and socket, up to 5 times; the arm fails as apparatus if none lingers. The no-linger control now also checks the probe reads a close with SO_LINGER off as returned.
FumingPower3925
added
engine/iouring
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: goceleris/celeris/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe Linux linger-close test checks whether a socket remains in its linger wait. It adds a no-linger control and retries linger attempts when the close has already returned. ChangesLinger-close test
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Other Merge Risk: ⚪ Minimal · up to The Linux linger-close test has no established issue that should block merging after normal checks. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
Summary
TestDriverLingeringCloseDoesNotStallTheWorker/lingerfailed in 3 of 30 linger runs on main since #744, which is 3 of 6 runs of the job "io_uring init-failure regression (./engine/iouring)" (not required). It also failed once on #746 and once on #768. Every failure was "L's onClose fired while its close still lingered", and in every one the close had not lingered at all:onclose_after_finalize_mswas 100.5 to 100.6, the same as the no-linger control, and V's byte arrived in under 0.2 ms. The engine was right; the test was wrong.Refs #763 (item 5).
The defect in the rig
The kernel ends a close's linger wait early when a signal is pending (
sk_stream_wait_closestops onsignal_pending). A Go test process gets signals. When one arrives, the engine'sclose(2)of its duplicate returns at once, and the fixed engine then firesonCloseat once, as it should. The rig read "onClose fired 100 ms after finalize" as "onClose fired while the close lingered". It had no way to tell "this close did not linger" from "onClose ran before a lingering close returned", which is the defect #744 guards against.Reproduced. The control OLDSIG1 is main's test, unchanged, plus one step: it sends SIGURG to the thread blocked in
close(opFD), which it finds through/proc/self/task/*/syscall. The Go runtime ignores a SIGURG it did not ask for. Main's test then fails 5 of 5 with CI's signature:onclose_before_v=true,onclose_after_finalize_ms100.6 to 102.2, V's byte in 0.04 to 0.52 ms, and the subtest over in 0.12 s.The fix (test only)
closeInLingerWaitlooks in/proc/net/tcpfor a row that still carries the socket's inode.tcp_closeorphans the socket, which sets that inode to 0, only when its linger wait ends, however it ends: the FIN is ACKed, the linger time runs out, or a signal arrives. Each attempt waits for the engine's close to begin (its number no longer names the socket), serves V, looks foronClose, and only then reads the probe. A close still waiting at that point was waiting while V was served and whileonClosewas looked for.celeris735 LINGER arm=linger attempt=N ... close_lingering=... verdict=valid|void|failfor every attempt, then one lineceleris735 LINGER arm=linger attempts=N void=M result=lingered|fail.onClosefired while the close is still in its linger wait is a FAIL. A V held past the 1 s budget is a FAIL at once, whether the close lingered or not.TestDriverShutdownWaitsForAHandedOffClose(#763 item 4) and the CI step's list are unchanged.Numbers
All runs are at 19106c1, in one container: linux/arm64, 4 CPUs, memlock 8 MiB (the step's shape), its own uid, kernel 7.0.12.
The fixed test, in CI's shape. The CI step "celeris#691 io_uring driver tests" ran exactly as
ci.ymlruns it (29 names,-race -count=5 -v), 20 times:TestDriverLingeringCloseDoesNotStallTheWorker: 100 PASS, 0 FAIL. All 100 linger arms lingered at their first attempt (attempts=1 void=0).onClosecame 2999.4 to 3062.9 ms after finalize.Controls. Each is a
go test -overlay, so the source is never edited. Each runs-race -count=5on this test alone:finalizeDriverfiresonCloseat once, and a goroutine only closes (re-cut on thisdriver.go)onclose_before_v=true close_lingering=false,onClose101.7 to 104.8 ms after finalize), attempt 2 lingeredA first run of the suite, at the same head, ran as root. 8 of its 20 step iterations skipped or failed on ring ENOMEM ("io_uring not available on this system"). io_uring charges ring memory to the uid, and another lane's root container was running io_uring engines at the same time. None of those failures was in the linger check. The whole suite was then run again as a uid of its own; those are the numbers above.
Evidence:
evidence/celeris-763-linger/. It hasmake_mutants.py,ctr.sh,in_container.sh,suite.shandsummary.sh, the logs inlogs/19106c1/with the tally inSUMMARY.txt, and the root run inlogs/19106c1-run1-root-enomem/. Every log starts with its worktree, head, porcelain, uid, shape and image.Test Plan
golangci-lintv2.13.2: 0 issues on./engine/iouring/...for GOOS=linux on amd64 and arm64, andgo vetis cleanTested on: [ ] std [ ] epoll [x] io_uring — [ ] amd64 [x] arm64
Release notes
breaking)testing: not user-facing)