From ec6e09014f9b244577acbb08ec99c8a72dde3e50 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Padez?= <andre.padez@gmail.com>
Date: Sat, 29 Aug 2026 11:32:25 +0100
Subject: [PATCH] a photograph may be slow, but it may not take the machine
 down
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

ImageMagick sizes its pixel cache against the machine and will hold an entire
decoded image in RAM. On a host with room that is right. On a small one it is
how resizing a photograph kills whatever else is running — and "whatever else"
includes the website this service exists to serve, which is not an acceptable
way for an image to fail.

So the decode runs under `-limit memory` / `-limit map` / `-limit disk`. Past
the caps ImageMagick spills to a temporary file and carries on, which turns an
out-of-memory kill of whatever the kernel picks into a conversion that is
slower. Nobody minds a slow photograph; everybody minds a site that went down
while one was being resized.

Measured on fofos. A 12 MP HEIC peaks at 145 MB, so the 256 MB default does not
bind and the output is byte-identical — it is a ceiling, not a tax. Forced to
bind, on a 36 MP image with a 32 MiB cap, peak RSS is 12 MB and it still exits 0
with the file written.

`-limit thread 1` for the same reason: one decode is one image, and each thread
carries its own share of the cache the caps exist to bound.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 sidecar/config.ts          | 15 +++++++++++++
 sidecar/sharp/decodable.ts | 43 +++++++++++++++++++++++++++++++-------
 2 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/sidecar/config.ts b/sidecar/config.ts
index a7cb60e..5e75ad7 100644
--- a/sidecar/config.ts
+++ b/sidecar/config.ts
@@ -65,6 +65,21 @@ export const WORK_ROOT = (() => {
 
 export const MAX_UPLOAD_BYTES = num("AV_CLUB_MAX_UPLOAD_BYTES", 512 * 1024 * 1024, 1024);
 
+/**
+ * How much RAM ImageMagick may hold while decoding, before it spills to a temporary file.
+ *
+ * Deliberately modest. This is the one operation here that can allocate against the size of the
+ * PICTURE rather than against anything this service chose, so it is the one that can take a small
+ * machine down — and taking a machine down is a worse outcome than being slow, every time, because
+ * the other things on it did not ask to be part of an image conversion.
+ *
+ * 256 MB comfortably holds the working set of a phone photograph. Past it nothing fails: ImageMagick
+ * memory-maps and then falls back to disk, so a genuinely enormous image is slower rather than fatal.
+ *
+ * Raise it on a machine with headroom. See sharp/decodable.ts for how it is applied.
+ */
+export const CONVERT_MEMORY_MB = num("AV_CLUB_CONVERT_MEMORY_MB", 256, 32);
+
 /** A crafted file can make an encoder run effectively forever. The process is killed at this point. */
 export const JOB_TIMEOUT_MS = num("AV_CLUB_JOB_TIMEOUT_MS", 120_000, 1_000);
 
diff --git a/sidecar/sharp/decodable.ts b/sidecar/sharp/decodable.ts
index b1a4cb3..e5dfe0c 100644
--- a/sidecar/sharp/decodable.ts
+++ b/sidecar/sharp/decodable.ts
@@ -1,7 +1,7 @@
 import { join } from "node:path";
 import { mkdtemp, rm } from "node:fs/promises";
 import { tmpdir } from "node:os";
-import { JOB_TIMEOUT_MS } from "../config";
+import { CONVERT_MEMORY_MB, JOB_TIMEOUT_MS } from "../config";
 import { unprocessable } from "../errors";
 
 /**
@@ -52,12 +52,41 @@ export async function decodableInput(path: string, format: string): Promise<Deco
   const workDir = await mkdtemp(join(tmpdir(), "avclub-heif-"));
   const converted = join(workDir, "decoded.png");
 
-  // ImageMagick 6 on Ubuntu 24.04, so `convert` — there is no `magick`. Spawned as an argv array,
-  // so the path is data rather than something a shell parses.
-  //
-  // -auto-orient applies the EXIF rotation while the metadata still exists; the PNG carries none, so
-  // this is the last chance to get a phone photograph the right way up.
-  const proc = Bun.spawn(["convert", path, "-auto-orient", converted], {
+  /*
+    ImageMagick 6 on Ubuntu 24.04, so `convert` — there is no `magick`. Spawned as an argv array, so
+    the path is data rather than something a shell parses.
+
+    `-auto-orient` applies the EXIF rotation while the metadata still exists; the PNG carries none, so
+    this is the last chance to get a phone photograph the right way up.
+
+    ── THE LIMITS ARE THE POINT, AND THEY GO FIRST ──
+
+    Left alone, ImageMagick sizes its pixel cache against the machine and will happily hold an entire
+    decoded image in RAM. On a box with room that is exactly right. On a small one it is how a
+    photograph takes down everything else on the host — and "everything else" here includes the website
+    this service exists to serve, which is not an acceptable way for an image to fail.
+
+    `-limit memory` caps the in-RAM cache; `-limit map` caps the memory-mapped extension of it. Past
+    both, ImageMagick spills to a temporary FILE and carries on. So the failure mode changes from an
+    out-of-memory kill of whatever the kernel picks to a conversion that is slower than it would have
+    been. That is a trade worth making unconditionally: nobody minds a slow photograph, and everybody
+    minds a website that went down while one was being resized.
+
+    These are DEFAULTS, not a policy — `AV_CLUB_CONVERT_MEMORY_MB` raises them on a machine with the
+    headroom to be quicker.
+  */
+  const limits = [
+    "-limit", "memory", `${CONVERT_MEMORY_MB}MiB`,
+    "-limit", "map", `${CONVERT_MEMORY_MB * 2}MiB`,
+    // The spill has to land somewhere, and unbounded is a full disk instead of a full memory. Ten
+    // times the memory cap is far more than any real photograph needs and still a number.
+    "-limit", "disk", `${CONVERT_MEMORY_MB * 10}MiB`,
+    // One decode is one image. Threads here buy little and each carries its own share of the cache,
+    // which is the opposite of what the caps above are for.
+    "-limit", "thread", "1",
+  ];
+
+  const proc = Bun.spawn(["convert", ...limits, path, "-auto-orient", converted], {
     stdout: "pipe",
     stderr: "pipe",
     stdin: "ignore",