Stop a transcode whose request has ended before it encodes (closes #243)

Process looked at its context only while it waited for a processing
slot, so a transcode went on decoding and encoding, holding its slot,
however long after its request's downstream_timeout that took.
acquireSlot now gives the slot back and returns the context's error
when the context has ended by the time it has a slot, and encode
returns that error without beginning. libvips cannot stop an encode
once it has begun, so that encode keeps its slot until it finishes.
README.md gives how long that can take per output format at the 8192
limits and no longer says the work stops when downstream_timeout ends;
TODO.md is corrected the same way.

Model: opus-5-5
This commit is contained in:
2026-10-15 07:44:23 +00:00
committed by sneak
parent 7789701831
commit 639a86a4a8
3 changed files with 65 additions and 24 deletions
+17 -3
View File
@@ -313,9 +313,23 @@ cache at once, they share one upstream fetch (or one read of the cached source)
and one transcode: the first request does the work, and the others wait for its and one transcode: the first request does the work, and the others wait for its
image or its error, holding no upstream connection or processing slot of their image or its error, holding no upstream connection or processing slot of their
own. A waiting request stops waiting when its own client goes away. The work own. A waiting request stops waiting when its own client goes away. The work
goes on for the others even if the first request's client goes away, until that goes on for the others even if the first request's client goes away. The shared
request's `downstream_timeout` ends. The shared fetch sends the first request's fetch sends the first request's ID upstream, and the lines logged for the fetch
ID upstream, and the lines logged for the fetch and the transcode carry that ID. and the transcode carry that ID.
A transcode whose request's `downstream_timeout` has ended (for a shared one,
the first request's) stops when it gets its processing slot or before it begins
encoding, and gives the slot back. libvips cannot stop an encode once it has
begun, so an encode keeps its slot until it finishes, which can be after
`downstream_timeout`. The limit of 8192 on each side of the source and of the
output bounds how long that takes. With one libvips thread, as pixad runs it,
encoding 8192x8192 random pixels takes up to about 1 second as JPEG, 15 as PNG,
16 as GIF, 63 as WebP, 110 as AVIF and 96 as JPEG XL, the slowest of 8 or 16
bits per sample, with or without an alpha channel. WebP, AVIF and JPEG XL are
that slow only with an alpha channel: 8-bit RGB takes about 44, 53 and 28
seconds. Decoding such a source adds up to about 11 seconds. A WebP, AVIF or
JPEG XL encode can therefore hold its slot past the default `downstream_timeout`
of 60 seconds.
The login form (`POST /`) is limited to 5 attempts per minute per client The login form (`POST /`) is limited to 5 attempts per minute per client
address, counting an IPv6 client by its /64; an attempt over the limit is address, counting an IPv6 client by its /64; an attempt over the limit is
+20 -5
View File
@@ -38,6 +38,21 @@ P2: security: per-origin rate limiting
and the query now have each `%` written as `%25` and each `:` as `%3A`; a path and the query now have each `%` written as `%25` and each `:` as `%3A`; a path
and query holding neither are signed and keyed as before. Signature and query holding neither are signed and keyed as before. Signature
Specification in `README.md` says so. Specification in `README.md` says so.
- 2026-10-15 a transcode stops once its request's `downstream_timeout` has
ended, unless it has begun encoding (closes #243): the image processor looked
at its context only while it waited for a processing slot, so an image was
decoded and encoded, holding the slot, however long after its request's
deadline that took. `Process` now returns the context's error, and gives the
slot back, when the context has ended by the time it has the slot or before it
encodes. libvips cannot stop an encode once it has begun, so that encode keeps
its slot until it finishes; the limit of 8192 on each side of the source and
of the output bounds how long that takes. With one libvips thread, 8192x8192
random pixels take up to about 1 second to encode as JPEG, 15 as PNG, 16 as
GIF, 63 as WebP, 110 as AVIF and 96 as JPEG XL, the last three with an alpha
channel, plus up to about 11 seconds to decode the source; `README.md` gives
these times and no longer says the work stops when `downstream_timeout` ends.
The settings of the WebP, AVIF and JPEG XL encoders, which can hold a slot
past the default `downstream_timeout` of 60 seconds, are unchanged.
- 2026-10-15 the example config allowlists no host (closes #249): it allowlisted - 2026-10-15 the example config allowlists no host (closes #249): it allowlisted
`github.com` and `user-images.githubusercontent.com`, where anyone can put an `github.com` and `user-images.githubusercontent.com`, where anyone can put an
image, so a deployment set up as Getting Started says served those images to image, so a deployment set up as Getting Started says served those images to
@@ -501,11 +516,11 @@ P2: security: per-origin rate limiting
cancellation but keeps its deadline, and the others wait for its image or cancellation but keeps its deadline, and the others wait for its image or
error holding no upstream connection or processing slot, and stop waiting when error holding no upstream connection or processing slot, and stop waiting when
their own context ends; the request doing the processing waits for it even their own context ends; the request doing the processing waits for it even
then, up to its deadline; a request whose context has already ended starts then, until it ends, which can be after its deadline; a request whose context
nothing; each request counts one miss, and the processing counts its fetch and has already ended starts nothing; each request counts one miss, and the
transcode once; a panic while processing is reported to Sentry when processing counts its fetch and transcode once; a panic while processing is
`sentry_dsn` is set and becomes an error for every waiting request instead of reported to Sentry when `sentry_dsn` is set and becomes an error for every
stopping pixad; documented in `README.md`. waiting request instead of stopping pixad; documented in `README.md`.
- 2026-09-29 only the image routes send CORS headers (closes #98): the CORS - 2026-09-29 only the image routes send CORS headers (closes #98): the CORS
middleware, with the `access_control_allow_origin` origin, moved from the middleware, with the `access_control_allow_origin` origin, moved from the
router root onto a `/v1` subrouter holding `/v1/image/` and `/v1/e/`, where it router root onto a `/v1` subrouter holding `/v1/image/` and `/v1/e/`, where it
+28 -16
View File
@@ -208,7 +208,9 @@ func New(params Params) *ImageProcessor {
// Process transforms an image according to the request. When // Process transforms an image according to the request. When
// MaxConcurrentProcessing images are already being processed, it waits up // MaxConcurrentProcessing images are already being processed, it waits up
// to ProcessingWaitTimeout for one to finish, then fails with // to ProcessingWaitTimeout for one to finish, then fails with
// ErrTooManyImages. // ErrTooManyImages. When ctx has ended once Process has a slot, or before it
// encodes, it returns ctx's error and gives the slot back. libvips cannot stop
// an encode once it has begun, so an encode keeps its slot until it finishes.
func (p *ImageProcessor) Process( func (p *ImageProcessor) Process(
ctx context.Context, ctx context.Context,
input io.Reader, input io.Reader,
@@ -290,7 +292,7 @@ func (p *ImageProcessor) Process(
} }
// Encode to target format // Encode to target format
output, err := p.encode(img, outputFormat, req.Quality) output, err := p.encode(ctx, img, outputFormat, req.Quality)
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to encode: %w", err) return nil, fmt.Errorf("failed to encode: %w", err)
} }
@@ -417,25 +419,28 @@ func (p *ImageProcessor) WaitForProcessing(ctx context.Context) int {
// acquireSlot takes a slot in processingSemaphore, waiting at most // acquireSlot takes a slot in processingSemaphore, waiting at most
// processingWaitTimeout for one to free up, and returns the func that gives // processingWaitTimeout for one to free up, and returns the func that gives
// it back. A free slot is taken even when ctx has ended; only the wait for // it back. When ctx has ended by the time it has a slot, it gives the slot
// one stops when ctx ends, as the rest of Process does not check ctx. // back and returns ctx's error.
func (p *ImageProcessor) acquireSlot(ctx context.Context) (func(), error) { func (p *ImageProcessor) acquireSlot(ctx context.Context) (func(), error) {
release := func() { <-p.processingSemaphore }
select { select {
case p.processingSemaphore <- struct{}{}: case p.processingSemaphore <- struct{}{}:
return release, nil
default:
}
select {
case p.processingSemaphore <- struct{}{}:
return release, nil
case <-time.After(p.processingWaitTimeout): case <-time.After(p.processingWaitTimeout):
return nil, ErrTooManyImages return nil, ErrTooManyImages
case <-ctx.Done(): case <-ctx.Done():
return nil, ctx.Err() return nil, ctx.Err()
} }
release := func() { <-p.processingSemaphore }
// select takes the slot even when ctx has ended at the same moment
err := ctx.Err()
if err != nil {
release()
return nil, err
}
return release, nil
} }
// detectFormat returns the format string for a govips image type. // detectFormat returns the format string for a govips image type.
@@ -527,10 +532,17 @@ func (p *ImageProcessor) resize(
const defaultQuality = 85 const defaultQuality = 85
// encode encodes an image to the specified format. // encode encodes an image to the specified format. libvips cannot stop an
// encode once it has begun, so when ctx has ended encode returns ctx's error
// without beginning one.
func (p *ImageProcessor) encode( func (p *ImageProcessor) encode(
img *vips.ImageRef, format Format, quality int, ctx context.Context, img *vips.ImageRef, format Format, quality int,
) ([]byte, error) { ) ([]byte, error) {
err := ctx.Err()
if err != nil {
return nil, err
}
if quality <= 0 { if quality <= 0 {
quality = defaultQuality quality = defaultQuality
} }
@@ -540,7 +552,7 @@ func (p *ImageProcessor) encode(
// libvips' built-in profile; govips' own sRGB path variable is set on // libvips' built-in profile; govips' own sRGB path variable is set on
// first use but read without a lock, so concurrent requests race on it. // first use but read without a lock, so concurrent requests race on it.
if img.HasICCProfile() { if img.HasICCProfile() {
err := img.TransformICCProfileWithFallback("srgb", "srgb") err = img.TransformICCProfileWithFallback("srgb", "srgb")
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to convert to sRGB: %w", err) return nil, fmt.Errorf("failed to convert to sRGB: %w", err)
} }