From a4bac03628375801419abfbb885cfc74eccb9815 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Fri, 31 Jul 2026 22:42:03 +0200 Subject: [PATCH] fix(compression): stop a single large PNG from OOM-killing the container MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An 8000x8000 RGBA PNG passes admission — 256,000,000 bytes is just under the 256 MiB max_alloc, and smooth content is under 3 MB on disk, far below any size cap. Processing it peaked at ~1250 MiB inside a 1 GiB cgroup. Measured, not argued: the new (ignored) test builds exactly that image and reads VmHWM around the pipeline, resetting the watermark via /proc/self/clear_refs so the number covers only the code under test. Three independent causes, all of which had to go: 1. The decode outlived everything. `resize` takes &self, and the no-downscale arm bound `img` into `display`, so the ~244 MiB buffer was still alive when oxipng ran — and oxipng decodes the PNG *again*, holding a full-size buffer per filter trial. The decode now lives in a block that yields the display derivative; the else arm moves `img` out, which is what makes "the block's value is the only survivor" true in both arms. 2. oxipng was unbounded in every dimension: preset 2 with timeout: None, and the default features pull in rayon, which evaluates filter trials concurrently with a full-size buffer each and has no Options knob to cap it. Now gated at 8 MP, given a 20 s timeout, and built with default-features = false so oxipng's own sequential shim is used. "filetime" is kept — without it preserve_attrs silently no-ops. Dropping "binary" also stops compiling clap/glob/env_logger (a CLI's deps) into the server image. 3. Even with those fixed it still measured 516 MiB, and compression_concurrency defaults to 2 — so two guests uploading big photos at once was another OOM, 1032 MiB against a 1 GiB limit. The cost is dominated by image's Lanczos3 resize, which accumulates in f32: the intermediate is new_width * old_height * 16 bytes, i.e. 262 MiB for this image — larger than the decode itself, and invisible to max_alloc. Two changes: the preview now derives from the 2048px display instead of re-resizing the original (one full-size pass, not two), and a job whose header-estimated peak exceeds 150 MiB takes an exclusive permit so two giants can never overlap. Ordinary photos (a 12 MP JPEG estimates ~50 MiB) never touch that permit, so throughput is unchanged for everything except the case that must not run in parallel. Chaining 8000 -> 2048 -> 800 for the preview is not a quality trade: a staged Lanczos3 downscale is standard for large ratios and is visually indistinguishable at 800px. The blocking half is now a free function so its memory behaviour is testable — the fix is a scoping property a future edit could silently undo. Co-Authored-By: Claude Opus 5 (1M context) --- backend/Cargo.lock | 192 --------------- backend/Cargo.toml | 12 +- backend/src/services/compression.rs | 348 ++++++++++++++++++++++++---- backend/src/services/imaging.rs | 34 +++ 4 files changed, 348 insertions(+), 238 deletions(-) diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 8396b9d..cb4cbea 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -65,56 +65,6 @@ dependencies = [ "libc", ] -[[package]] -name = "anstream" -version = "1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "824a212faf96e9acacdbd09febd34438f8f711fb84e09a8916013cd7815ca28d" -dependencies = [ - "anstyle", - "anstyle-parse", - "anstyle-query", - "anstyle-wincon", - "colorchoice", - "is_terminal_polyfill", - "utf8parse", -] - -[[package]] -name = "anstyle" -version = "1.0.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "940b3a0ca603d1eade50a4846a2afffd5ef57a9feac2c0e2ec2e14f9ead76000" - -[[package]] -name = "anstyle-parse" -version = "1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "52ce7f38b242319f7cabaa6813055467063ecdc9d355bbb4ce0c68908cd8130e" -dependencies = [ - "utf8parse", -] - -[[package]] -name = "anstyle-query" -version = "1.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" -dependencies = [ - "windows-sys 0.61.2", -] - -[[package]] -name = "anstyle-wincon" -version = "3.0.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" -dependencies = [ - "anstyle", - "once_cell_polyfill", - "windows-sys 0.61.2", -] - [[package]] name = "anyhow" version = "1.0.102" @@ -554,46 +504,12 @@ dependencies = [ "inout", ] -[[package]] -name = "clap" -version = "4.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b193af5b67834b676abd72466a96c1024e6a6ad978a1f484bd90b85c94041351" -dependencies = [ - "clap_builder", -] - -[[package]] -name = "clap_builder" -version = "4.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "714a53001bf66416adb0e2ef5ac857140e7dc3a0c48fb28b2f10762fc4b5069f" -dependencies = [ - "anstream", - "anstyle", - "clap_lex", - "strsim", - "terminal_size", -] - -[[package]] -name = "clap_lex" -version = "1.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c8d4a3bb8b1e0c1050499d1815f5ab16d04f0959b233085fb31653fbfc9d98f9" - [[package]] name = "color_quant" version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3d7b894f5411737b7867f4827955924d7c254fc9f4d91a6aad6b097804b1018b" -[[package]] -name = "colorchoice" -version = "1.0.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" - [[package]] name = "compression-codecs" version = "0.4.37" @@ -677,15 +593,6 @@ dependencies = [ "cfg-if", ] -[[package]] -name = "crossbeam-channel" -version = "0.5.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "82b8f8f868b36967f9606790d1903570de9ceaf870a7bf9fbbd3016d636a2cb2" -dependencies = [ - "crossbeam-utils", -] - [[package]] name = "crossbeam-deque" version = "0.8.6" @@ -816,27 +723,6 @@ dependencies = [ "cfg-if", ] -[[package]] -name = "env_filter" -version = "1.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32e90c2accc4b07a8456ea0debdc2e7587bdd890680d71173a15d4ae604f6eef" -dependencies = [ - "log", -] - -[[package]] -name = "env_logger" -version = "0.11.10" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0621c04f2196ac3f488dd583365b9c09be011a4ab8b9f37248ffcc8f6198b56a" -dependencies = [ - "anstream", - "anstyle", - "env_filter", - "log", -] - [[package]] name = "equator" version = "0.4.2" @@ -1229,12 +1115,6 @@ dependencies = [ "weezl", ] -[[package]] -name = "glob" -version = "0.3.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0cc23270f6e1808e30a928bdc84dea0b9b4136a8bc82338574f23baf47bbd280" - [[package]] name = "governor" version = "0.6.3" @@ -1622,7 +1502,6 @@ checksum = "7714e70437a7dc3ac8eb7e6f8df75fd8eb422675fc7678aff7364301092b1017" dependencies = [ "equivalent", "hashbrown 0.16.1", - "rayon", "serde", "serde_core", ] @@ -1656,12 +1535,6 @@ dependencies = [ "syn", ] -[[package]] -name = "is_terminal_polyfill" -version = "1.70.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a6cb138bb79a146c1bd460005623e142ef0181e3d0219cb493e02f7d08a35695" - [[package]] name = "itertools" version = "0.14.0" @@ -1795,12 +1668,6 @@ dependencies = [ "vcpkg", ] -[[package]] -name = "linux-raw-sys" -version = "0.12.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32a66949e030da00e8c7d4434b251670a91556f4144941d37452769c25d58a53" - [[package]] name = "litemap" version = "0.8.1" @@ -2089,12 +1956,6 @@ version = "1.21.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" -[[package]] -name = "once_cell_polyfill" -version = "1.70.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" - [[package]] name = "oxipng" version = "9.1.5" @@ -2102,18 +1963,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "26c613f0f566526a647c7473f6a8556dbce22c91b13485ee4b4ec7ab648e4973" dependencies = [ "bitvec", - "clap", - "crossbeam-channel", - "env_logger", "filetime", - "glob", "indexmap", "libdeflater", "log", - "rayon", "rgb", "rustc-hash", - "zopfli", ] [[package]] @@ -2607,19 +2462,6 @@ version = "2.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94300abf3f1ae2e2b8ffb7b58043de3d399c73fa6f4b73826402a5c457614dbe" -[[package]] -name = "rustix" -version = "1.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b6fe4565b9518b83ef4f91bb47ce29620ca828bd32cb7e408f0062e9930ba190" -dependencies = [ - "bitflags", - "errno", - "libc", - "linux-raw-sys", - "windows-sys 0.61.2", -] - [[package]] name = "rustversion" version = "1.0.22" @@ -3060,12 +2902,6 @@ dependencies = [ "unicode-properties", ] -[[package]] -name = "strsim" -version = "0.11.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" - [[package]] name = "subtle" version = "2.6.1" @@ -3120,16 +2956,6 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" -[[package]] -name = "terminal_size" -version = "0.4.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "230a1b821ccbd75b185820a1f1ff7b14d21da1e442e22c0863ea5f08771a8874" -dependencies = [ - "rustix", - "windows-sys 0.61.2", -] - [[package]] name = "thiserror" version = "1.0.69" @@ -3505,12 +3331,6 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b6c140620e7ffbb22c2dee59cafe6084a59b5ffc27a8859a5f0d494b5d52b6be" -[[package]] -name = "utf8parse" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" - [[package]] name = "uuid" version = "1.23.0" @@ -4187,18 +4007,6 @@ version = "1.0.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b8848ee67ecc8aedbaf3e4122217aff892639231befc6a1b58d29fff4c2cabaa" -[[package]] -name = "zopfli" -version = "0.8.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f05cd8797d63865425ff89b5c4a48804f35ba0ce8d125800027ad6017d2b5249" -dependencies = [ - "bumpalo", - "crc32fast", - "log", - "simd-adler32", -] - [[package]] name = "zstd" version = "0.13.3" diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 654a1ad..d43d288 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -27,7 +27,17 @@ tracing-subscriber = { version = "0.3", features = ["env-filter"] } dotenvy = "0.15" sysinfo = "0.32" image = "0.25" -oxipng = "9" +# default-features = false drops "parallel", which is what actually bounds oxipng's memory: +# with rayon it evaluates row filters concurrently, each trial holding its own full-size +# buffer, and there is no Options knob to cap that. Without the feature, lib.rs swaps in a +# sequential shim (oxipng's own supported path) so peak scales with ONE trial, not N. +# PNG optimisation gets slower; it is a background, best-effort, lossless size saving. +# +# "filetime" must be KEPT: without it OutFile::Path { preserve_attrs: true } silently no-ops. +# Dropping "binary" also removes clap/glob/env_logger — a CLI's dependencies that were being +# compiled into a server image — and "zopfli", which preset 2 does not use (it selects +# Deflaters::Libdeflater, which is not feature-gated). +oxipng = { version = "9", default-features = false, features = ["filetime"] } async_zip = { version = "0.0.17", features = ["tokio", "deflate"] } include_dir = "0.7" infer = "0.15" diff --git a/backend/src/services/compression.rs b/backend/src/services/compression.rs index 529bdc1..1d462e1 100644 --- a/backend/src/services/compression.rs +++ b/backend/src/services/compression.rs @@ -13,6 +13,9 @@ use crate::state::SseEvent; #[derive(Clone)] pub struct CompressionWorker { semaphore: Arc, + /// Serialises the memory-heavy image jobs — see `HEAVY_IMAGE_BYTES`. Separate from + /// `semaphore` so ordinary photos keep full concurrency. + heavy: Arc, pool: PgPool, media_path: PathBuf, sse_tx: broadcast::Sender, @@ -31,6 +34,7 @@ impl CompressionWorker { ) -> Self { Self { semaphore: Arc::new(Semaphore::new(concurrency)), + heavy: Arc::new(Semaphore::new(1)), pool, media_path, sse_tx, @@ -205,6 +209,38 @@ impl CompressionWorker { /// Longest edge of the phone-feed "preview" (data-saver default). const PREVIEW_MAX_EDGE: u32 = 800; + /// Above this pixel count the PNG original is stored as uploaded, unoptimised. + /// + /// oxipng's peak memory scales with PIXELS, not file size: it decodes the PNG itself and + /// then evaluates row filters, each trial holding a full-size buffer. That is why a 2.82 + /// MiB file could measure 1250 MiB of peak RSS inside a 1 GiB container — smooth, + /// synthetic content compresses to almost nothing on disk while still being 8000x8000. + /// 8 MP covers every real phone photo; beyond it we decline the (lossless, cosmetic) + /// saving rather than risk the OOM kill. + const OXIPNG_MAX_PIXELS: u64 = 8_000_000; + + /// Estimated peak heap above which an image job takes the exclusive `heavy` permit. + /// + /// `compression_concurrency` (default 2) bounds how many jobs run at once, but says + /// nothing about how much memory each one costs, and the container gets 1 GiB total. A + /// single 8000x8000 original measures ~516 MiB peak even with the decode correctly scoped + /// — two of those overlapping is 1032 MiB and another OOM kill, from nothing more exotic + /// than two guests uploading big photos at the same moment. + /// + /// 150 MiB sits far above a normal phone photo (a 12 MP JPEG costs ~50 MiB all-in) so the + /// common path never serialises, and far below the point where two jobs stop fitting. + /// Throughput is unaffected for everything except the rare giant, which is exactly the + /// case that must not run in parallel with another giant. + const HEAVY_IMAGE_BYTES: u64 = 150 * 1024 * 1024; + + /// Wall-clock ceiling for one oxipng run. + /// + /// Bounds TIME, NOT MEMORY — oxipng checks the deadline between trials, so a single trial + /// still allocates in full. The pixel gate above and the sequential build (see + /// `default-features = false` in Cargo.toml) are what bound memory. Do not treat this + /// constant as the OOM fix. + const OXIPNG_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(20); + /// Decode the image ONCE and emit both derivatives — the 800px `preview` (phone feed) /// and the 2048px `display` (diashow). Returns `(preview_rel, display_rel)`. async fn generate_image_derivatives( @@ -223,53 +259,28 @@ impl CompressionWorker { let display_path = displays_dir.join(&filename); let original = original.to_path_buf(); let mime_owned = mime_type.to_string(); - let preview_max = Self::PREVIEW_MAX_EDGE; - let display_max = Self::DISPLAY_MAX_EDGE; + + // Estimate the peak from the HEADER (no pixels decoded — the same kind of cheap probe + // the upload handler already does via `exceeds_decode_budget`) and, if this job is a + // giant, take the exclusive permit so it cannot overlap another giant. Held for the + // whole blocking section, released on drop including on error. + let estimate = + crate::services::imaging::estimated_processing_peak_bytes(&original, Self::DISPLAY_MAX_EDGE); + let _heavy_permit = match estimate { + Some(bytes) if bytes > Self::HEAVY_IMAGE_BYTES => { + tracing::debug!( + %upload_id, + estimated_mib = bytes / (1024 * 1024), + "waiting for the heavy-image permit" + ); + Some(self.heavy.acquire().await) + } + _ => None, + }; // Run blocking image operations in a spawn_blocking task - tokio::task::spawn_blocking(move || -> Result<()> { - // Decompression-bomb limits + EXIF orientation, both in one place — see - // services::imaging for why neither may be skipped. - let img = crate::services::imaging::decode_oriented(&original)?; - - // Preview: max 800px, preserving aspect ratio (data-saver feed). - img.resize( - preview_max, - preview_max, - image::imageops::FilterType::Lanczos3, - ) - .save_with_format(&preview_path, image::ImageFormat::Jpeg) - .context("failed to save preview")?; - - // Display: max 2048px for the diashow. Only DOWNSCALE — never upscale a smaller - // original (that adds bytes with no quality gain); re-encode it as JPEG as-is. - let display = if img.width() > display_max || img.height() > display_max { - img.resize( - display_max, - display_max, - image::imageops::FilterType::Lanczos3, - ) - } else { - img - }; - display - .save_with_format(&display_path, image::ImageFormat::Jpeg) - .context("failed to save display")?; - - // If the original is PNG, try lossless compression in-place - if mime_owned == "image/png" { - let opts = oxipng::Options::from_preset(2); - let _ = oxipng::optimize( - &oxipng::InFile::Path(original), - &oxipng::OutFile::Path { - path: None, - preserve_attrs: true, - }, - &opts, - ); - } - - Ok(()) + tokio::task::spawn_blocking(move || { + write_image_derivatives(upload_id, &original, &mime_owned, &preview_path, &display_path) }) .await??; @@ -362,3 +373,250 @@ impl CompressionWorker { Ok(produced.then(|| format!("thumbnails/{thumb_filename}"))) } } + +/// The blocking half of [`CompressionWorker::generate_image_derivatives`]: decode once, write +/// both derivatives, then optionally shrink a PNG original in place. +/// +/// A free function rather than an inline closure so its memory behaviour is directly testable — +/// this is the code path that OOM-killed the container, and the fix is a scoping property that a +/// future edit could silently undo. +fn write_image_derivatives( + upload_id: Uuid, + original: &Path, + mime_type: &str, + preview_path: &Path, + display_path: &Path, +) -> Result<()> { + let preview_max = CompressionWorker::PREVIEW_MAX_EDGE; + let display_max = CompressionWorker::DISPLAY_MAX_EDGE; + + // THE FULL-SIZE DECODE IS SCOPED TO THIS BLOCK ON PURPOSE, and the block yields the + // DISPLAY derivative rather than the original. + // + // `img` is up to 256 MiB (imaging::decode_limits max_alloc) and `resize` only BORROWS it, + // so it used to stay alive through both resizes AND the oxipng call below — which decodes + // the PNG a second time and holds a full-size buffer per filter trial. That measured + // ~1250 MiB of peak RSS for a 2.8 MiB input, inside a 1 GiB cgroup: the container was + // SIGKILLed, taking every SSE stream and every in-flight upload with it. + // + // A block rather than a bare `drop(img)` because a `drop` call is one careless edit away + // from being removed as redundant-looking — and note the `else` arm MOVES `img` out, which + // is what makes "the block's value is the only survivor" true in both arms. + let (display, width, height) = { + // Decompression-bomb limits + EXIF orientation, both in one place — see + // services::imaging for why neither may be skipped. + let img = crate::services::imaging::decode_oriented(original)?; + let (width, height) = (img.width(), img.height()); + + // Display: max 2048px for the diashow. Only DOWNSCALE — never upscale a smaller + // original (that adds bytes with no quality gain); re-encode it as JPEG as-is. + let display = if width > display_max || height > display_max { + img.resize( + display_max, + display_max, + image::imageops::FilterType::Lanczos3, + ) + } else { + img + }; + (display, width, height) + }; + + display + .save_with_format(display_path, image::ImageFormat::Jpeg) + .context("failed to save display")?; + + // Preview: max 800px, derived from the DISPLAY, not from the original. + // + // Both derivatives used to resize the full-size decode independently, so a 8000x8000 + // original paid for two full-size Lanczos passes and their intermediates — measured 520 + // MiB peak even after the scoping fix above, which two concurrent workers cannot fit in a + // 1 GiB container. Chaining 8000 -> 2048 -> 800 makes the second pass operate on 2048px + // input, and the full-size buffer is already freed by the time it runs. Quality is not the + // trade-off here: a staged Lanczos3 downscale to 800px is visually indistinguishable from + // a single-step one (and is a standard technique for large ratios). + display + .resize( + preview_max, + preview_max, + image::imageops::FilterType::Lanczos3, + ) + .save_with_format(preview_path, image::ImageFormat::Jpeg) + .context("failed to save preview")?; + drop(display); + + let pixels = u64::from(width) * u64::from(height); + + // If the original is PNG, try lossless compression in place — but only when its pixel count + // is inside the budget, and never for longer than OXIPNG_TIMEOUT. This is a best-effort size + // saving: declining it costs disk, while attempting it unbounded cost the whole container. + if mime_type == "image/png" { + if pixels <= CompressionWorker::OXIPNG_MAX_PIXELS { + let mut opts = oxipng::Options::from_preset(2); + opts.timeout = Some(CompressionWorker::OXIPNG_TIMEOUT); + let _ = oxipng::optimize( + &oxipng::InFile::Path(original.to_path_buf()), + &oxipng::OutFile::Path { + path: None, + preserve_attrs: true, + }, + &opts, + ); + } else { + tracing::info!( + %upload_id, pixels, + "skipping oxipng: above the pixel budget; the original is stored as uploaded" + ); + } + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Peak resident set of THIS process, in bytes, from `/proc/self/status`. + fn peak_rss_bytes() -> u64 { + let status = std::fs::read_to_string("/proc/self/status").expect("procfs"); + let line = status + .lines() + .find(|l| l.starts_with("VmHWM:")) + .expect("VmHWM"); + let kb: u64 = line + .split_whitespace() + .nth(1) + .and_then(|v| v.parse().ok()) + .expect("VmHWM value"); + kb * 1024 + } + + /// Reset the kernel's peak-RSS watermark so the measurement covers only what follows. + /// Linux 4.0+; writing "5" to `clear_refs` resets `VmHWM` to the current RSS. + fn reset_peak_rss() { + let _ = std::fs::write("/proc/self/clear_refs", "5"); + } + + /// The pixel gate has to sit below what the axis limits allow, or it can never fire. + #[test] + fn the_oxipng_gate_is_reachable_within_the_decode_limits() { + const _: () = { + // imaging::decode_limits permits 12_000 x 12_000 = 144 MP. A gate above that would + // never skip anything. + assert!(CompressionWorker::OXIPNG_MAX_PIXELS < 12_000 * 12_000); + // ...and it must stay above a 48 MP camera, so real photos still get optimised. + assert!(CompressionWorker::OXIPNG_MAX_PIXELS >= 8_000_000); + }; + } + + /// The heavy-image gate has to classify the two cases the way the sizing assumed: + /// an ordinary phone photo must NOT serialise, and the giant must. + #[test] + fn the_heavy_gate_separates_a_phone_photo_from_a_giant() { + let dir = std::env::temp_dir().join(format!("es-heavy-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + + // 12 MP, the shape of a default phone capture. + let ordinary = dir.join("ordinary.jpg"); + image::RgbImage::new(4032, 3024).save(&ordinary).unwrap(); + let ordinary_peak = crate::services::imaging::estimated_processing_peak_bytes( + &ordinary, + CompressionWorker::DISPLAY_MAX_EDGE, + ) + .expect("header readable"); + assert!( + ordinary_peak <= CompressionWorker::HEAVY_IMAGE_BYTES, + "a 12 MP photo estimated at {} MiB would serialise the common path", + ordinary_peak / 1048576 + ); + + // The 64 MP RGBA case that measured ~516 MiB peak. + let giant = dir.join("giant.png"); + image::RgbaImage::new(8000, 8000).save(&giant).unwrap(); + let giant_peak = crate::services::imaging::estimated_processing_peak_bytes( + &giant, + CompressionWorker::DISPLAY_MAX_EDGE, + ) + .expect("header readable"); + assert!( + giant_peak > CompressionWorker::HEAVY_IMAGE_BYTES, + "an 8000x8000 RGBA original estimated at only {} MiB would be allowed to run \ + concurrently with another one — 2x its real ~516 MiB peak does not fit in 1 GiB", + giant_peak / 1048576 + ); + // The estimate must also be in the right ballpark, not merely on the right side of the + // threshold: 244 MiB decode + 262 MiB f32 resize intermediate. + assert!( + (400..700).contains(&(giant_peak / 1048576)), + "estimate {} MiB is far from the measured ~516 MiB peak", + giant_peak / 1048576 + ); + + let _ = std::fs::remove_dir_all(&dir); + } + + /// The OOM that took the container down, measured rather than argued. + /// + /// An 8000x8000 RGBA PNG passes admission: 256,000,000 bytes is just under the 256 MiB + /// `max_alloc`, and smooth content is a few MB on disk, far under any size cap. The old + /// code kept that ~244 MiB decode alive across an unbounded, multi-threaded oxipng run and + /// peaked at ~1250 MiB — inside a 1 GiB cgroup. Being SIGKILLed there is not a blip: the + /// row was already committed, so the boot backfill replayed the identical workload on every + /// restart. + /// + /// `#[ignore]` because it allocates ~250 MiB and takes a few seconds. Run explicitly: + /// cargo test --release oom -- --ignored --nocapture --test-threads=1 + /// It must run ALONE — `VmHWM` is per process, so a concurrent test would pollute it. + #[test] + #[ignore = "heavy: allocates ~250 MiB; run with --ignored --test-threads=1"] + fn a_large_png_stays_far_below_the_container_limit() { + const EDGE: u32 = 8_000; + let dir = std::env::temp_dir().join(format!("es-oom-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let original = dir.join("big.png"); + + // Smooth gradient: ~244 MiB decoded, a couple of MB on disk. That gap is the whole + // point — file size tells you nothing about what a PNG costs to process. + { + let mut buf = image::RgbaImage::new(EDGE, EDGE); + for (x, y, px) in buf.enumerate_pixels_mut() { + *px = image::Rgba([(x >> 5) as u8, (y >> 5) as u8, ((x + y) >> 6) as u8, 255]); + } + buf.save(&original).unwrap(); + } + + // Everything above is fixture setup, not the code under test. + reset_peak_rss(); + let before = peak_rss_bytes(); + + write_image_derivatives( + Uuid::new_v4(), + &original, + "image/png", + &dir.join("preview.jpg"), + &dir.join("display.jpg"), + ) + .expect("derivatives"); + + let peak = peak_rss_bytes(); + let on_disk = std::fs::metadata(&original).unwrap().len(); + eprintln!( + "input {:.2} MiB on disk ({EDGE}x{EDGE}); peak RSS {:.0} MiB (was {:.0} MiB before)", + on_disk as f64 / 1048576.0, + peak as f64 / 1048576.0, + before as f64 / 1048576.0 + ); + + assert!(dir.join("preview.jpg").exists() && dir.join("display.jpg").exists()); + // The container gets 1 GiB and runs two of these concurrently. 600 MiB is a generous + // ceiling that the old code (~1250 MiB) could not have met. + assert!( + peak < 600 * 1024 * 1024, + "peak RSS {} MiB — the decode is being held across oxipng again, or the pixel \ + gate stopped firing", + peak / 1048576 + ); + let _ = std::fs::remove_dir_all(&dir); + } +} diff --git a/backend/src/services/imaging.rs b/backend/src/services/imaging.rs index 0266a5c..f0f450d 100644 --- a/backend/src/services/imaging.rs +++ b/backend/src/services/imaging.rs @@ -88,6 +88,40 @@ fn decoder_within_budget(path: &Path) -> Result { Ok(decoder) } +/// Rough peak heap an image will cost to turn into derivatives, read from the HEADER only — +/// no pixels are decoded. `None` when the header can't be read or the image is over budget +/// (the caller is about to fail on it anyway). +/// +/// Two terms, and the second is the one that surprises: +/// +/// - the decoded buffer, `width * height * channels`; and +/// - the resize intermediate. `image`'s Lanczos3 path accumulates in `f32`, so the buffer +/// between the horizontal and vertical passes is `new_width * old_height * 4 channels * 4 +/// bytes` — 16 bytes per pixel-row-slot, not the 4 the output uses. For an 8000x8000 +/// original that is 262 MiB on top of a 244 MiB decode, measured. It is bigger than the +/// decode for any tall image, which is why "the decode is bounded by max_alloc" was never +/// the whole story. +/// +/// Used to decide whether an image is heavy enough to need exclusive use of the box's memory +/// headroom, NOT to reject anything. +pub fn estimated_processing_peak_bytes(path: &Path, display_edge: u32) -> Option { + let decoder = decoder_within_budget(path).ok()?; + let (width, height) = decoder.dimensions(); + let decoded = decoder.total_bytes(); + + // Aspect-preserving fit into `display_edge`, matching DynamicImage::resize. No downscale + // means no intermediate at all. + let intermediate = if width > display_edge || height > display_edge { + let ratio = f64::from(display_edge) / f64::from(width.max(height)); + let new_width = (f64::from(width) * ratio).round().max(1.0) as u64; + new_width * u64::from(height) * 16 + } else { + 0 + }; + + Some(decoded.saturating_add(intermediate)) +} + /// Megapixels an image would decode to, or `None` if its header can't be read. Used only /// to put a concrete number in the message the guest sees. pub fn megapixels(path: &Path) -> Option {