You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Follow-ups from the review of #730 (#705, part of #706, item 1 of #716). None of them blocks the merge: the maintainer capped reviews at two rounds on 2026-09-27, and these are minors.
read-waiting-in-window uses a sleep as synchronisation (CodeRabbit, minor; middleware/websocket/engineread_latespill_test.go, the third case of TestChanReaderSpillPublishedAfterDrain, around lines 298-313). The window callback starts the async Read and then calls time.Sleep(50 * time.Millisecond) so that the Read reaches its wait decision before the chunk is published. On a loaded host the sleep can end before the Read starts. The chunk is then published first, and the case passes without exercising the in-window interleaving. The case's comment already says so: it can pass wrongly and cannot fail wrongly. Suggested fix: replace the sleep with waitLockWaiter(t, "(*chanReader).promoteSpill"), which TestChanReaderCloseKeepsChunksAppendedBeforeIt already uses. With the fix, next finds r.ch empty and parks in promoteSpill on spillMu, which the publisher holds. Without the fix, the Read parks on r.ch, and waitLockWaiter fails after 10 s, which is the negative control. waitLockWaiter must release r.spillMu before it calls t.Fatalf, for example through an onFail callback. Otherwise the test goroutine exits holding the lock and the parked Read leaks. Update the comment so it describes the lock-based wait and not a 50 ms budget. Kill check: mutant N3 from fix(websocket): never strand a chunk that spills after the handler drained the channel (celeris#705); fail the start helpers fast with Start's error (celeris#706) #730's body (the promotion behind the lock-free spillLen check) must still fail this case every time.
The helpers' own _ = ln.Close() and Shutdown still do no harm, because both releases are idempotent. They are now a second line of defence, not the only release. Reword the comments, and decide whether each explicit release stays as a guard. The readiness-timeout path, where Start is still running when the wait gives up, is the case where the helper's cancel and wait still matter.
Follow-ups from the review of #730 (#705, part of #706, item 1 of #716). None of them blocks the merge: the maintainer capped reviews at two rounds on 2026-09-27, and these are minors.
read-waiting-in-windowuses a sleep as synchronisation (CodeRabbit, minor;middleware/websocket/engineread_latespill_test.go, the third case ofTestChanReaderSpillPublishedAfterDrain, around lines 298-313). The window callback starts the asyncReadand then callstime.Sleep(50 * time.Millisecond)so that theReadreaches its wait decision before the chunk is published. On a loaded host the sleep can end before theReadstarts. The chunk is then published first, and the case passes without exercising the in-window interleaving. The case's comment already says so: it can pass wrongly and cannot fail wrongly. Suggested fix: replace the sleep withwaitLockWaiter(t, "(*chanReader).promoteSpill"), whichTestChanReaderCloseKeepsChunksAppendedBeforeItalready uses. With the fix,nextfindsr.chempty and parks inpromoteSpillonspillMu, which the publisher holds. Without the fix, theReadparks onr.ch, andwaitLockWaiterfails after 10 s, which is the negative control.waitLockWaitermust releaser.spillMubefore it callst.Fatalf, for example through anonFailcallback. Otherwise the test goroutine exits holding the lock and the parkedReadleaks. Update the comment so it describes the lock-based wait and not a 50 ms budget. Kill check: mutant N3 from fix(websocket): never strand a chunk that spills after the handler drained the channel (celeris#705); fail the start helpers fast with Start's error (celeris#706) #730's body (the promotion behind the lock-freespillLencheck) must still fail this case every time.The start-helper comments go stale once fix(server): a Start that never serves releases the caller's listener, the CPU monitor and the settle re-opener (#737) #747 (server: a Start that fails before its engine runs leaves the caller's listener and the CPU monitor's /proc/stat descriptor open #737) is on the branch (found by the merge train when it brought in
main). fix(server): a Start that never serves releases the caller's listener, the CPU monitor and the settle re-opener (#737) #747 made aStartthat fails before its engine runs close the caller's listener (prepareWithListener), and madedoPrepareclose the CPU monitor whencreateEnginefails. These comments still say that a failedStartcloses neither:middleware/websocket/engine_linux_test.go:80-83(startNativeServerWithHandle)middleware/websocket/engine_start_test.go:56-57middleware/static/start_helper_linux_test.go:49-50middleware/static/retained_key_linux_test.go:105The helpers' own
_ = ln.Close()andShutdownstill do no harm, because both releases are idempotent. They are now a second line of defence, not the only release. Reword the comments, and decide whether each explicit release stays as a guard. The readiness-timeout path, whereStartis still running when the wait gives up, is the case where the helper's cancel and wait still matter.