From de6f5e34ea63b4807a65c42c625504523a004e3d Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 08:28:26 +0000 Subject: [PATCH] Keep application/octet-stream out of memory for variants (closes #70) VariantStorage.LoadWithMeta now returns an empty content type when the .meta file is missing or unreadable, and GetVariant serves application/octet-stream in that case without keeping it in memory. Memory now only ever holds a content type read from a .meta file or passed to StoreVariant, so a read that ran before a store wrote the .meta file can no longer leave the wrong type in memory, however it interleaves with the store, other reads and eviction. Model: opus-5-5 --- TODO.md | 13 +++++++------ internal/imgcache/cache.go | 16 ++++++++++++---- internal/imgcache/eviction.go | 4 ++-- internal/imgcache/storage.go | 7 ++++--- 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/TODO.md b/TODO.md index 0fabcd6..31268f1 100644 --- a/TODO.md +++ b/TODO.md @@ -34,12 +34,13 @@ exhaustion `Cache.metaCache` holds the content types of up to 10,000 variants in an LRU (`github.com/hashicorp/golang-lru/v2`), filled by `StoreVariant` and by `GetVariant` after it reads a `.meta` file, where a type `StoreVariant` added - meanwhile is kept over the one read; for a variant it holds, `GetVariant` - skips the `.meta` read, still opening the variant file and taking the size - from it; eviction removes the entry before deleting the files, and - `GetVariant` removes it when the file will not open; the cap is a constant, - not a setting; the unused `variantMeta` type is gone; `README.md` describes - it. + meanwhile is kept over the one read, and never with the + `application/octet-stream` served for a variant without one; for a variant it + holds, `GetVariant` skips the `.meta` read, still opening the variant file and + taking the size from it; eviction removes the entry before deleting the files, + and `GetVariant` removes it when the file will not open; the cap is a + constant, not a setting; the unused `variantMeta` type is gone; `README.md` + describes it. - 2026-09-29 Dockerfiles install through `script/bootstrap` (closes #95): the `Dockerfile` lint and build stages and `Dockerfile.lint` copy `script/`, `go.mod` and `go.sum`, then run `script/bootstrap` in place of their own diff --git a/internal/imgcache/cache.go b/internal/imgcache/cache.go index 1e81452..61632b6 100644 --- a/internal/imgcache/cache.go +++ b/internal/imgcache/cache.go @@ -184,7 +184,9 @@ func (c *Cache) Lookup(ctx context.Context, req *ImageRequest) (*LookupResult, e // GetVariant returns a reader, size, and content type for a cached // variant. The content type comes from metaCache, or else from the -// variant's .meta file and is then kept in metaCache. +// variant's .meta file and is then kept in metaCache. A variant with +// no .meta file is served as application/octet-stream, which is not +// kept. func (c *Cache) GetVariant(cacheKey VariantKey) (io.ReadCloser, int64, string, error) { if c.disabled { return nil, 0, "", ErrNotFound @@ -521,9 +523,11 @@ func (c *Cache) IncrementTransformCount(ctx context.Context) { // loadVariantWithMeta is GetVariant for a variant metaCache does not // hold: it reads the content type from the variant's .meta file and // keeps it in metaCache, unless a StoreVariant has put one there -// meanwhile. The stored one wins, since this read may have found the -// variant file before the store wrote the .meta file, and so got -// application/octet-stream. +// meanwhile, as the store's is newer. A read that finds no .meta file, +// as one can between a store's writing of the variant file and of its +// .meta file, serves application/octet-stream and keeps nothing, so +// metaCache only ever holds a type read from a .meta file or passed to +// StoreVariant. func (c *Cache) loadVariantWithMeta( cacheKey VariantKey, ) (io.ReadCloser, int64, string, error) { @@ -532,6 +536,10 @@ func (c *Cache) loadVariantWithMeta( return nil, 0, "", err } + if contentType == "" { + return reader, size, fallbackContentType, nil + } + c.metaCache.ContainsOrAdd(cacheKey, contentType) return reader, size, contentType, nil diff --git a/internal/imgcache/eviction.go b/internal/imgcache/eviction.go index 9366334..5d185ea 100644 --- a/internal/imgcache/eviction.go +++ b/internal/imgcache/eviction.go @@ -39,8 +39,8 @@ const tempFilePrefix = ".tmp-" // to each variant file. const variantMetaSuffix = ".meta" -// fallbackContentType is recorded when a reconciled variant file has -// no readable .meta sidecar. +// fallbackContentType is the content type given to a variant file that +// has no readable .meta sidecar, when it is served or reconciled. const fallbackContentType = "application/octet-stream" // UsageBytes returns the total number of bytes of cache content diff --git a/internal/imgcache/storage.go b/internal/imgcache/storage.go index 2ecabca..7e1cb38 100644 --- a/internal/imgcache/storage.go +++ b/internal/imgcache/storage.go @@ -531,7 +531,8 @@ func (s *VariantStorage) LoadWithSize(key VariantKey) (io.ReadCloser, int64, err } // LoadWithMeta returns a reader, size, and content type for the content at -// the given key. +// the given key. The content type is read from the .meta file, and is +// empty when that file is missing or unreadable. func (s *VariantStorage) LoadWithMeta( key VariantKey, ) (io.ReadCloser, int64, string, error) { @@ -540,8 +541,8 @@ func (s *VariantStorage) LoadWithMeta( return nil, 0, "", err } - // Load metadata for content type - contentType := "application/octet-stream" // fallback + var contentType string + metaPath := s.keyToPath(key) + ".meta" metaData, err := os.ReadFile(metaPath) //nolint:gosec // path derived from cache key