Pull requests / #3546

#3546 serve: a cached image file is never deleted while a request still needs it

open · @usakim97 · 0 commentaires · Sur GitHub

needs-format

Description

## The failure

A request that sends images encodes them under the FIFO turn, then copies the cached
`.sve` files into one combined file **after** the turn is released. Another request can
take the turn in between, encode enough pictures to evict the first request's entries
(the cache keeps 64), and unlink those files. The first request's
`write_temporary(combined, ...)` then raises `FileNotFoundError` on `read_bytes()`.

Nothing catches it: the handler dies before writing a response, so the client sees a
dropped connection — behind a reverse proxy, `502` with an empty body. It reproduces
with a long conversation that resends 30+ frames per turn: measured on a 140k-token
session, every request failed in 2.7 s while the same server answered 200 to curl with
146k-token prompts, 41 MB bodies, images, tools, streaming and h2.

## The fix

- `Vision.hold(paths)` / `Vision.release(paths)`: a request holds the files it has
  encoded from inside the turn that encoded them until its combined file is written.
  Eviction skips held files. This closes the window completely: no request can evict a
  file another request has not copied yet.
- A cache hit whose file is gone is not a hit. `encode()` checks `path.is_file()` and
  encodes the picture again instead of returning a path the caller cannot read (a
  Windows temp cleaner can also remove one; `_ensure_dir` already covers the whole
  directory disappearing).
- `release()` ends the hold, so the next eviction may take the file.

## Tests

`serve/test_vision_cache.py` — 6 tests, no GPU, a fake encoder writes the `.sve`:
a hit is reused; a hit whose file is gone re-encodes; a held file survives 199 evictions
and is evicted after release; a request keeps its own files while it encodes 100
pictures; the map stays at 64; the combined file holds every picture.

`serve.test_server.VisionCacheEviction` keeps passing (its helper gains the new
`in_use` field). Full `serve` suite: 899 tests, the only failure is
`test_fatal_recovery.test_http_health_and_next_request_recovery`, which fails the same
way on unmodified `main` on this machine.

## Note, not changed here

The 64-entry cache is small for a frame-review session: every turn re-encodes its
pictures. Correctness no longer depends on the size, so this PR leaves the bound alone.

Sur le site

Liens install, modèles, releases.