HandleLogout in internal/handlers/api.go deletes the client, counts the session's remaining clients and, at zero, calls cleanupUser, which is BroadcastQuit in internal/service/service.go. The three steps are separate statements, so when two clients of the same session log out at the same time both deletes run before either count, both see zero, and BroadcastQuit runs twice: every channel peer gets two QUIT messages from the nick. Two concurrent HTTP QUIT commands do the same, since handleQuit calls BroadcastQuit straight away and the second call still finds the channels unless the first has parted them.
Seen on eceb5f2: a session with a password and two clients, both sending POST /api/v1/logout at once; the peer in the shared channel received two QUITs in nine of nine trials. README.md says the QUIT is broadcast to the users who share a channel; a client shows the user quitting twice.
Fix: make the teardown run once. BroadcastQuit claims the session before it does anything, with one statement whose result says whether it won (a DELETE FROM sessions WHERE id = ? reported through rows affected, after reading the channel list and the peers in the same transaction), and a caller that did not win sends nothing. HandleLogout's delete and count belong in that transaction too.
Definition of done
Concurrent logouts or QUITs of one session produce one QUIT per peer; the session, its clients and its memberships are deleted either way.
A test under -race with two concurrent logouts and one with two concurrent QUITs, each checking that the peer's queue holds one QUIT.
One reviewed PR against next.
Model: fable-5-1
`HandleLogout` in `internal/handlers/api.go` deletes the client, counts the session's remaining clients and, at zero, calls `cleanupUser`, which is `BroadcastQuit` in `internal/service/service.go`. The three steps are separate statements, so when two clients of the same session log out at the same time both deletes run before either count, both see zero, and `BroadcastQuit` runs twice: every channel peer gets two QUIT messages from the nick. Two concurrent HTTP `QUIT` commands do the same, since `handleQuit` calls `BroadcastQuit` straight away and the second call still finds the channels unless the first has parted them.
Seen on `eceb5f2`: a session with a password and two clients, both sending `POST /api/v1/logout` at once; the peer in the shared channel received two QUITs in nine of nine trials. `README.md` says the QUIT is broadcast to the users who share a channel; a client shows the user quitting twice.
Fix: make the teardown run once. `BroadcastQuit` claims the session before it does anything, with one statement whose result says whether it won (a `DELETE FROM sessions WHERE id = ?` reported through rows affected, after reading the channel list and the peers in the same transaction), and a caller that did not win sends nothing. `HandleLogout`'s delete and count belong in that transaction too.
## Definition of done
1. Concurrent logouts or QUITs of one session produce one QUIT per peer; the session, its clients and its memberships are deleted either way.
2. A test under `-race` with two concurrent logouts and one with two concurrent QUITs, each checking that the peer's queue holds one QUIT.
3. One reviewed PR against `next`.
Model: fable-5-1
clawbot
self-assigned this 2026-10-20 17:14:51 +02:00
Also in scope, found by the review at #164 (comment): BroadcastQuit parts a session's channels and deletes the emptied ones as two statements, so a JOIN racing the last member's QUIT can land between them, find the channel with no members, and join as its operator, skipping the ban, +i, +k and +l checks of a channel that keeps them. The fix here parts and deletes each channel in one transaction, as #164 does for PART and KICK (PartChannelAndDeleteIfEmpty), inside the transaction that claims the session. Definition of done gains: a test where a JOIN races the last member's QUIT on a +k channel and never joins without the key.
Model: opus-5-5
Also in scope, found by the review at https://git.eeqj.de/sneak/neoirc/pulls/164#issuecomment-152068: `BroadcastQuit` parts a session's channels and deletes the emptied ones as two statements, so a JOIN racing the last member's QUIT can land between them, find the channel with no members, and join as its operator, skipping the ban, `+i`, `+k` and `+l` checks of a channel that keeps them. The fix here parts and deletes each channel in one transaction, as https://git.eeqj.de/sneak/neoirc/pulls/164 does for PART and KICK (`PartChannelAndDeleteIfEmpty`), inside the transaction that claims the session. Definition of done gains: a test where a JOIN races the last member's QUIT on a `+k` channel and never joins without the key.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
HandleLogoutininternal/handlers/api.godeletes the client, counts the session's remaining clients and, at zero, callscleanupUser, which isBroadcastQuitininternal/service/service.go. The three steps are separate statements, so when two clients of the same session log out at the same time both deletes run before either count, both see zero, andBroadcastQuitruns twice: every channel peer gets two QUIT messages from the nick. Two concurrent HTTPQUITcommands do the same, sincehandleQuitcallsBroadcastQuitstraight away and the second call still finds the channels unless the first has parted them.Seen on
eceb5f2: a session with a password and two clients, both sendingPOST /api/v1/logoutat once; the peer in the shared channel received two QUITs in nine of nine trials.README.mdsays the QUIT is broadcast to the users who share a channel; a client shows the user quitting twice.Fix: make the teardown run once.
BroadcastQuitclaims the session before it does anything, with one statement whose result says whether it won (aDELETE FROM sessions WHERE id = ?reported through rows affected, after reading the channel list and the peers in the same transaction), and a caller that did not win sends nothing.HandleLogout's delete and count belong in that transaction too.Definition of done
-racewith two concurrent logouts and one with two concurrent QUITs, each checking that the peer's queue holds one QUIT.next.Model: fable-5-1
Also in scope, found by the review at #164 (comment):
BroadcastQuitparts a session's channels and deletes the emptied ones as two statements, so a JOIN racing the last member's QUIT can land between them, find the channel with no members, and join as its operator, skipping the ban,+i,+kand+lchecks of a channel that keeps them. The fix here parts and deletes each channel in one transaction, as #164 does for PART and KICK (PartChannelAndDeleteIfEmpty), inside the transaction that claims the session. Definition of done gains: a test where a JOIN races the last member's QUIT on a+kchannel and never joins without the key.Model: opus-5-5