Commit Graph
4 Commits
Author SHA1 Message Date
developer b6d70a50fe Finish the server shutdown tests; fix the two bugs they found
build / build-push (push) Successful in 22s
internal/server was the last package whose shutdown was tested by assumption:
four transports, three shutdown shapes, and every one of them can strand a
goroutine that a client is holding open. Both bugs below were live in the tree
as handed over and neither had a test.

An idle client could hold the whole shutdown open. The stream accept loop waits
for its connections before Stop returns, and a connection parked in ReadMsg with
no deadline in front of it only ends when the client goes away — ctx.Err() is
checked between reads, so cancelling the context cannot reach a read that is
already blocked. One connection that connected and said nothing was enough to
make Stop sit there for the caller's entire grace period (15s in main) and then
return context.DeadlineExceeded, which the app reports as "shutdown completed
with errors"; and the cache snapshot the app takes right after Stop never
happened, because the orchestrator had given up first. It is a shutdown DoS that
costs the attacker one connection. The fix is a read deadline in the past on
every accepted connection when the context is cancelled, and deliberately not a
close: a query already inside the pipeline is past its read, so its answer still
goes out, which is the invariant TestInFlightQueryIsAnsweredAcrossStop pins. The
accept loop keeps a small registry of live connections for this — the accept
path, not the query path, and no query ever takes that mutex.

A UDP query in flight at Stop was lost and nothing waited for it. The stream
paths register their connections with s.wait; servePacket did not register at
all, so a UDP query already inside the pipeline was gone twice over: nothing
waited for it, and the answer had nowhere to go once it was ready, because a
datagram is written to the very socket the query arrived on. The visible damage
was not the lost answer (the client retries) but the app, which snapshots the
cache and closes the store the moment Stop returns — a query still in the
pipeline could be writing to a store on its way out. Packets are now counted in
s.wait, and the UDP listener's close wakes the read loop with an expired
deadline and then waits, bounded by udpDrainGrace (250ms), for the loop and the
packets it spawned before closing the socket. The bound is deliberate: a
shutdown may not spend its grace period on one slow upstream, and past the bound
the answer is dropped, which is what the client would see anyway.

Stop returning did not mean the process was done, so nothing could tell a
leaking shutdown from a clean one.
TestStopLeavesNoGoroutineRunningServerCode counts goroutines whose stack still
has a frame in the package — not runtime.NumGoroutine(), which in a test binary
is net/http, crypto/tls and the race detector talking over each other — and it
found the idle-connection bug above. TestStopIsNotBlockedByAnIdleStreamConnection
sets a 60s DoT idle timeout on purpose so a config value cannot hide the
regression behind a lucky tick, and TestInFlightQueryIsAnsweredAcrossStop now
covers all four transports. harness.idleStream exists because a test that only
dials a stream transport races the accept loop: Stop can close the listener
while the connection is still in the backlog, the server never accepts it, and a
shutdown assertion that needs a live server-side goroutine passes for the wrong
reason.

AGENTS.md gains the two durable invariants (no step in the shutdown may wait for
a client; every query goroutine is registered with s.wait, UDP included) and the
two test-hygiene traps. STATUS.md drops the UDP query lost at Stop from its
pinned list, since it is now a test rather than an accident, and the plan starts
at the pinned decisions that are actually left.
2026-09-27 19:28:40 +00:00
developer e8c5b414d9 half broken
build / build-push (push) Failing after 23s
2026-09-27 17:17:53 +00:00
developer 4f49033f1b Cover the engine, config.Size and metrics; fix the five bugs they found
internal/engine, internal/config and internal/metrics had no test file at all,
and writing them turned up five real bugs.

internal/engine (23 tests, new harness_test.go): a wired engine over a fake
upstream on loopback, a local zone, a blocklist, a cache, a query log and an
injectable clock. It pins the pipeline order end to end -- ACL deny before the
local zone, local zone before blocklist, blocklist before cache, drop sends
nothing, the concurrency and rate-limit refusals, serve-stale served with TTL 1
and refreshed once per key, truncation against a 512-byte client, and exactly
one metrics record and one query-log entry per query.

Five fixes:

- config.ParseSize ignored the unit multiplier, so 512MiB parsed as 512 bytes
  and the whole memory envelope was inert for anyone who wrote a unit.

- finalize (EDNS0 and the UDP size) ran only on the cache-hit path, so a large
  uncached answer left oversized for a 512-byte client. It now runs on the
  upstream, local-zone and blocklist paths too.

- The upstream UDP client read buffer was hard-coded to 1232 while we advertise
  dns.max_udp_size, so with a larger buffer configured every answer above 1232
  bytes was read truncated and became SERVFAIL. The pool takes the size now.

- LRU.prefetchGate was a 64-slot global cap, not the single flight its comment
  promised: a burst of stale hits launched up to 64 duplicate refreshes of one
  name. It is a per-key latch now; distinct names still refresh in parallel.

- metrics.RecordMiss and RecordStaleHit moved only the Prometheus collector, so
  the Cache page's misses and stale hits read 0 for the life of the process.

AGENTS.md gains the trap that cost the most time: this version of miekg/dns
reports a character-string over 255 bytes as zero length, so such a message
silently fails to pack, and the write failure looks like a client timeout.
2026-09-27 16:03:21 +00:00
developer bbd8692bab Add AGENTS.md; bring STATUS.md up to a green tree
ci / Console typecheck and build (push) Successful in 13s
ci / Go build, vet and test (push) Successful in 20s
ci / Binary with the console embedded (push) Successful in 18s
ci / Container image (push) Failing after 7s
AGENTS.md is the part that should not churn: the settled decisions with their
reasons, the invariants that hold the design together, the working rules, and
the security-sensitive areas that have been wrong before. A new agent should
read it and STATUS.md and know what not to re-litigate.

Deliberately excluded: anything about how to search, read or patch. Those are
being automated per round, and writing advice about them into the repo would
rot. What is here instead is project-specific and cannot be automated away —
for example that a PTR suffix must match on a label boundary, that a truncated
answer must never be cached, and that the UDP read deadline has to be reset every
iteration or the loop spins.

The working rules are the part worth arguing for. Never leave the tree red,
because nobody can detect the budget running out: three of five subagents left
broken files, and each cost a round to discover. Report honestly, and when a test
fails, decide first whether the test, the implementation or the specification is
wrong — all three happen here, and the decision belongs in the commit message.

STATUS.md now records a fully green tree: nine packages with suites, all passing
under -race. The plan starts with internal/engine, which is the largest
uncovered surface and the one every packet goes through.
2026-09-27 14:17:14 +00:00