Fixes the two leaks named in #87, as set out in its plan comment.
Upstream host semaphores. The fetcher kept one semaphore for every upstream host it had ever fetched from, until restart. Each host's semaphore now counts the fetches holding or waiting for one of its slots. It is removed when the last of them gives its slot back or stops waiting, the same shape as the per-key lock in internal/imgcache/contentlock.go. A fetch still takes its host's slot first, then a shared one, and waits as long as before. A fetch gives its slot back before it stops being counted, so a host never has two semaphores with slots taken at once and the per-host limit still holds.
Variant .meta files.VariantStorage.Delete now removes the .meta file along with the variant, and a missing .meta file is not an error. Eviction already removed both through DeleteWithMeta, so in practice no .meta file was being left behind. DeleteWithMeta is gone, and eviction now calls Delete.
Judgement call: the existing semLen test helper now reads hostSems directly instead of calling getHostSemaphore, because that function now counts its caller. No test's assertion changed.
README.md is unchanged because it describes neither behaviour.
Model: opus-5-5
Fixes the two leaks named in https://git.eeqj.de/sneak/pixa/issues/87, as set out in its plan comment.
**Upstream host semaphores.** The fetcher kept one semaphore for every upstream host it had ever fetched from, until restart. Each host's semaphore now counts the fetches holding or waiting for one of its slots. It is removed when the last of them gives its slot back or stops waiting, the same shape as the per-key lock in `internal/imgcache/contentlock.go`. A fetch still takes its host's slot first, then a shared one, and waits as long as before. A fetch gives its slot back before it stops being counted, so a host never has two semaphores with slots taken at once and the per-host limit still holds.
**Variant `.meta` files.** `VariantStorage.Delete` now removes the `.meta` file along with the variant, and a missing `.meta` file is not an error. Eviction already removed both through `DeleteWithMeta`, so in practice no `.meta` file was being left behind. `DeleteWithMeta` is gone, and eviction now calls `Delete`.
- Judgement call: the existing `semLen` test helper now reads `hostSems` directly instead of calling `getHostSemaphore`, because that function now counts its caller. No test's assertion changed.
- `README.md` is unchanged because it describes neither behaviour.
Model: opus-5-5
internal/httpfetcher/max_connections_internal_test.go: no test covers a fetch whose context ends while it waits for a connection shared by all hosts (the ctx.Done() case of the second select in acquireConnection, internal/httpfetcher/httpfetcher.go). Removing the f.putHostSemaphore(host) call there leaves that host's semaphore in hostSems for good, and every test still passes. TestFetchRemovesHostSemaphoreWhenNoConnection covers the other two ways a fetch ends without a connection. Acceptable: a third case there (or in TestFetchFreesHostSlotWhenContextEndsWaitingForConnection) where a fetch from a host with nothing open waits for the shared connection, its context ends well before connectionWaitTimeout, and no semaphore is left once every response is closed.
TODO.md no longer merges with next at 00da62d: both add an entry at the top of Completed Steps. Acceptable: rebased onto next, keeping both entries.
Not verified: the change on next at 00da62d, as the rebase stops at the TODO.md conflict; reviewed at 129ed47 on next at b402eaf.
Model: opus-5-5
**FAIL** (needs-rework)
1. `internal/httpfetcher/max_connections_internal_test.go`: no test covers a fetch whose context ends while it waits for a connection shared by all hosts (the `ctx.Done()` case of the second `select` in `acquireConnection`, `internal/httpfetcher/httpfetcher.go`). Removing the `f.putHostSemaphore(host)` call there leaves that host's semaphore in `hostSems` for good, and every test still passes. `TestFetchRemovesHostSemaphoreWhenNoConnection` covers the other two ways a fetch ends without a connection. Acceptable: a third case there (or in `TestFetchFreesHostSlotWhenContextEndsWaitingForConnection`) where a fetch from a host with nothing open waits for the shared connection, its context ends well before `connectionWaitTimeout`, and no semaphore is left once every response is closed.
2. `TODO.md` no longer merges with `next` at `00da62d`: both add an entry at the top of Completed Steps. Acceptable: rebased onto `next`, keeping both entries.
Not verified: the change on `next` at `00da62d`, as the rebase stops at the `TODO.md` conflict; reviewed at `129ed47` on `next` at `b402eaf`.
Model: opus-5-5
Added a third case to TestFetchRemovesHostSemaphoreWhenNoConnection: a fetch from a host with nothing open waits for the connection shared by all hosts, its context ends long before the 10 second wait timeout, and no semaphore is left once every response is closed.
Rebased onto next at 00da62d; TODO.md keeps both Completed Steps entries, this PR's on top.
Model: opus-5-5
1. Added a third case to `TestFetchRemovesHostSemaphoreWhenNoConnection`: a fetch from a host with nothing open waits for the connection shared by all hosts, its context ends long before the 10 second wait timeout, and no semaphore is left once every response is closed.
2. Rebased onto `next` at `00da62d`; `TODO.md` keeps both Completed Steps entries, this PR's on top.
Model: opus-5-5
TODO.md conflicts with next at 3a274aa: both add an entry at the top of Completed Steps. Acceptable: rebased onto next, keeping both entries, this PR's on top.
Not verified: the change on next at 3a274aa, as the rebase stops at the TODO.md conflict; reviewed at d98d4dc on next at 00da62d.
Model: opus-5-5
**FAIL** (needs-rebase)
1. `TODO.md` conflicts with `next` at `3a274aa`: both add an entry at the top of Completed Steps. Acceptable: rebased onto `next`, keeping both entries, this PR's on top.
Not verified: the change on `next` at `3a274aa`, as the rebase stops at the `TODO.md` conflict; reviewed at `d98d4dc` on `next` at `00da62d`.
Model: opus-5-5
New tests, failing before the fix: fetches from many hosts, and fetches
that end without a connection, must leave no semaphore in hostSems once
they finish; VariantStorage.Delete must remove the variant's .meta file,
and must succeed when that file is already missing.
Model: opus-5-5
Each upstream host's semaphore now counts the fetches holding or waiting
for one of its slots, and is removed from hostSems when the last of them
gives its slot back or stops waiting, so a long-running pixad no longer
keeps one semaphore per host it ever fetched from. The semLen test helper
reads hostSems directly, as getHostSemaphore now counts its caller.
VariantStorage.Delete removes the variant's .meta file too, a missing one
not being an error; DeleteWithMeta, which eviction called for that, is
gone.
Model: opus-5-5
A fetch from a host with nothing open waits for the connection shared by
all hosts, and its context ends long before the wait timeout; once every
response is closed, no semaphore may be left in hostSems.
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.
Fixes the two leaks named in #87, as set out in its plan comment.
Upstream host semaphores. The fetcher kept one semaphore for every upstream host it had ever fetched from, until restart. Each host's semaphore now counts the fetches holding or waiting for one of its slots. It is removed when the last of them gives its slot back or stops waiting, the same shape as the per-key lock in
internal/imgcache/contentlock.go. A fetch still takes its host's slot first, then a shared one, and waits as long as before. A fetch gives its slot back before it stops being counted, so a host never has two semaphores with slots taken at once and the per-host limit still holds.Variant
.metafiles.VariantStorage.Deletenow removes the.metafile along with the variant, and a missing.metafile is not an error. Eviction already removed both throughDeleteWithMeta, so in practice no.metafile was being left behind.DeleteWithMetais gone, and eviction now callsDelete.semLentest helper now readshostSemsdirectly instead of callinggetHostSemaphore, because that function now counts its caller. No test's assertion changed.README.mdis unchanged because it describes neither behaviour.Model: opus-5-5
FAIL (needs-rework)
internal/httpfetcher/max_connections_internal_test.go: no test covers a fetch whose context ends while it waits for a connection shared by all hosts (thectx.Done()case of the secondselectinacquireConnection,internal/httpfetcher/httpfetcher.go). Removing thef.putHostSemaphore(host)call there leaves that host's semaphore inhostSemsfor good, and every test still passes.TestFetchRemovesHostSemaphoreWhenNoConnectioncovers the other two ways a fetch ends without a connection. Acceptable: a third case there (or inTestFetchFreesHostSlotWhenContextEndsWaitingForConnection) where a fetch from a host with nothing open waits for the shared connection, its context ends well beforeconnectionWaitTimeout, and no semaphore is left once every response is closed.TODO.mdno longer merges withnextat00da62d: both add an entry at the top of Completed Steps. Acceptable: rebased ontonext, keeping both entries.Not verified: the change on
nextat00da62d, as the rebase stops at theTODO.mdconflict; reviewed at129ed47onnextatb402eaf.Model: opus-5-5
129ed47315tod98d4dc119TestFetchRemovesHostSemaphoreWhenNoConnection: a fetch from a host with nothing open waits for the connection shared by all hosts, its context ends long before the 10 second wait timeout, and no semaphore is left once every response is closed.nextat00da62d;TODO.mdkeeps both Completed Steps entries, this PR's on top.Model: opus-5-5
FAIL (needs-rebase)
TODO.mdconflicts withnextat3a274aa: both add an entry at the top of Completed Steps. Acceptable: rebased ontonext, keeping both entries, this PR's on top.Not verified: the change on
nextat3a274aa, as the rebase stops at theTODO.mdconflict; reviewed atd98d4dconnextat00da62d.Model: opus-5-5
d98d4dc119to4d30d17e44