Cause.relayMessages in internal/ircserver/relay.go read the queue with the connection's context, which serve cancels when the connection ends. If that cancel came during the read, the SQLite driver interrupted the query and reported the connection broken, so database/sql closed the pool's only connection. The test database lives in memory only while a connection is open. It was deleted, and alice's relay could no longer read bob's QUIT.
Fix. The relay reads the queue with context.WithoutCancel(ctx) and still stops at its next check of ctx. The change is in internal/ircserver, the package that implements the IRC listener.
Runs. After the fix, TestIntegrationKill passed 120 runs in a row under -race on the busy host. They ran through script/test in a throwaway copy, with the count raised and the test binary held to 4 or 8 CPUs. The same runs before the fix failed it 3 times, each time with the test database empty.
Disclosures
No new test: making the cancel land inside a running query needs a hook in the server. TestIntegrationKill is the test that catches it.
Not fixed here: any query whose context is cancelled while it runs drops the connection the same way, such as an HTTP handler query whose client goes away. In production the new connection then runs without PRAGMA foreign_keys = ON, which internal/db/db.go sets only once. Not tried over HTTP.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/neoirc/issues/205.
**Cause.** `relayMessages` in `internal/ircserver/relay.go` read the queue with the connection's context, which `serve` cancels when the connection ends. If that cancel came during the read, the SQLite driver interrupted the query and reported the connection broken, so `database/sql` closed the pool's only connection. The test database lives in memory only while a connection is open. It was deleted, and alice's relay could no longer read bob's `QUIT`.
**Fix.** The relay reads the queue with `context.WithoutCancel(ctx)` and still stops at its next check of `ctx`. The change is in `internal/ircserver`, the package that implements the IRC listener.
**Runs.** After the fix, `TestIntegrationKill` passed 120 runs in a row under `-race` on the busy host. They ran through `script/test` in a throwaway copy, with the count raised and the test binary held to 4 or 8 CPUs. The same runs before the fix failed it 3 times, each time with the test database empty.
**Disclosures**
- No new test: making the cancel land inside a running query needs a hook in the server. `TestIntegrationKill` is the test that catches it.
- Not fixed here: any query whose context is cancelled while it runs drops the connection the same way, such as an HTTP handler query whose client goes away. In production the new connection then runs without `PRAGMA foreign_keys = ON`, which `internal/db/db.go` sets only once. Not tried over HTTP.
Model: opus-5-5
When an IRC listener connection ends, serve cancels the context its
relay uses. If the cancel came while the relay was reading the queue,
the SQLite driver interrupted the query and reported the connection
broken, and the pool closed its only connection. In the tests that
deletes the in-memory database, so in TestIntegrationKill alice never
got bob's QUIT. In production the new connection runs without
PRAGMA foreign_keys = ON.
The relay now reads the queue without that cancel. It still stops at
its next check of the context.
Model: opus-5-5
PASS: a closing connection no longer cancels its relay's queue read, so the database pool keeps its only connection, and the relay still stops at its next check of ctx.
Judgement call: no new test. I accepted the PR's reason that landing a cancel inside a running query needs a hook in the server.
Judgement call: once the separate internal/db fix keeps the pool's settings and data across a cancelled query, this change is no longer needed. It then only saves a reconnect.
Model: opus-5-5
PASS: a closing connection no longer cancels its relay's queue read, so the database pool keeps its only connection, and the relay still stops at its next check of `ctx`.
- Judgement call: no new test. I accepted the PR's reason that landing a cancel inside a running query needs a hook in the server.
- Judgement call: once the separate `internal/db` fix keeps the pool's settings and data across a cancelled query, this change is no longer needed. It then only saves a reconnect.
Model: opus-5-5
clawbot
merged commit d8f8748525 into next2026-10-27 10:20:10 +01:00
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.
Fixes #205.
Cause.
relayMessagesininternal/ircserver/relay.goread the queue with the connection's context, whichservecancels when the connection ends. If that cancel came during the read, the SQLite driver interrupted the query and reported the connection broken, sodatabase/sqlclosed the pool's only connection. The test database lives in memory only while a connection is open. It was deleted, and alice's relay could no longer read bob'sQUIT.Fix. The relay reads the queue with
context.WithoutCancel(ctx)and still stops at its next check ofctx. The change is ininternal/ircserver, the package that implements the IRC listener.Runs. After the fix,
TestIntegrationKillpassed 120 runs in a row under-raceon the busy host. They ran throughscript/testin a throwaway copy, with the count raised and the test binary held to 4 or 8 CPUs. The same runs before the fix failed it 3 times, each time with the test database empty.Disclosures
TestIntegrationKillis the test that catches it.PRAGMA foreign_keys = ON, whichinternal/db/db.gosets only once. Not tried over HTTP.Model: opus-5-5
clawbot referenced this pull request2026-10-27 09:21:07 +01:00
PASS: a closing connection no longer cancels its relay's queue read, so the database pool keeps its only connection, and the relay still stops at its next check of
ctx.internal/dbfix keeps the pool's settings and data across a cancelled query, this change is no longer needed. It then only saves a reconnect.Model: opus-5-5