From 69a7d535b7fdffb8861d565b9adad43dbc933441 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 8 Oct 2026 09:50:04 +0000 Subject: [PATCH] :bug: Keep pending SVG requests from blocking image and font requests The queue middleware is shared by every route, and a request holds its queue slot for its whole lifetime. An SVG request waiting for a free worker therefore occupied a global slot, and image and font requests queued behind it even though the SVG work runs on its own threads. SVG now has its own queue, so its waits only affect other SVG requests. Image and font keep sharing one queue. The app wiring moves to `app.ts` (`createApp`), apart from `index.ts` (which loads the config, wires the services and listens), so a test can build the real app without starting a server; `queue-isolation.test.ts` uses it to prove a pending SVG request does not block an image request. AI-assisted-by: deepseek-v4.1-flash --- .serena/memories/media-processor/core.md | 2 +- media-processor/src/app.ts | 37 ++++++++++ media-processor/src/index.ts | 23 +----- media-processor/test/queue-isolation.test.ts | 74 ++++++++++++++++++++ 4 files changed, 114 insertions(+), 22 deletions(-) create mode 100644 media-processor/src/app.ts create mode 100644 media-processor/test/queue-isolation.test.ts diff --git a/.serena/memories/media-processor/core.md b/.serena/memories/media-processor/core.md index d9a8ad0fad..022744b5ae 100644 --- a/.serena/memories/media-processor/core.md +++ b/.serena/memories/media-processor/core.md @@ -62,7 +62,7 @@ media-processor/ - SVG: max input size enforced before parsing (default 2MB, `PENPOT_MEDIA_PROCESSOR_SVG_MAX_SIZE`); an oversized input is rejected with 413, never truncated - SVG workers: `PENPOT_MEDIA_PROCESSOR_SVG_WORKERS` (default 2; 0 runs inline), `PENPOT_MEDIA_PROCESSOR_SVG_WORKER_MAX_OLD_MB` (default 512, per-worker V8 old-generation cap) and `PENPOT_MEDIA_PROCESSOR_SVG_TIMEOUT` (default 30000 ms) - Font: prlimit wraps FontForge processes with memory (AS) and CPU time limits -- Concurrency: p-queue limits concurrent requests (default 10) +- Concurrency: p-queue limits concurrent requests (default 10). Image and font share one queue; SVG has **its own**, so an SVG request waiting for a free worker does not hold a queue slot that image or font requests need. The app wiring lives in `app.ts` (`createApp`), kept apart from `index.ts` so tests can build the real app. - Upload: hybrid storage — memory for files < 10MB, disk for larger; configurable via `PENPOT_MEDIA_PROCESSOR_MEMORY_THRESHOLD` - Max file size: configurable (default 350MB) diff --git a/media-processor/src/app.ts b/media-processor/src/app.ts new file mode 100644 index 0000000000..cdc98d0306 --- /dev/null +++ b/media-processor/src/app.ts @@ -0,0 +1,37 @@ +import express, { type Express } from "express"; +import { healthRoutes } from "./routes/health.js"; +import { createImageRoutes } from "./routes/image.js"; +import { createFontRoutes } from "./routes/font.js"; +import { createSvgRoutes } from "./routes/svg.js"; +import { errorHandler } from "./middleware/error-handler.js"; +import { timeoutMiddleware } from "./middleware/timeout.js"; +import { sharedKeyAuth } from "./middleware/auth.js"; +import { createQueueMiddleware } from "./middleware/queue.js"; +import { loggingMiddleware } from "./middleware/logging.js"; +import type { AppConfig } from "./types.js"; + +/** + * Build the HTTP app. Kept apart from `index.ts` (which loads the config, wires + * the services and listens) so tests can build the real app without starting a + * server. + */ +export function createApp(config: Pick): Express { + const app = express(); + + // Image and font share one queue. SVG gets its own: an SVG request waiting for + // a free worker holds its queue slot for its whole lifetime, and on a shared + // queue that wait would block image and font requests behind it. + const queueMiddleware = createQueueMiddleware(config.maxConcurrentRequests); + const svgQueueMiddleware = createQueueMiddleware(config.maxConcurrentRequests); + + app.use(timeoutMiddleware(config.requestTimeout)); + app.use(loggingMiddleware); + + app.get("/api/health", healthRoutes); + app.use("/api/image", sharedKeyAuth(config.sharedKey), queueMiddleware, createImageRoutes()); + app.use("/api/font", sharedKeyAuth(config.sharedKey), queueMiddleware, createFontRoutes()); + app.use("/api/svg", sharedKeyAuth(config.sharedKey), svgQueueMiddleware, createSvgRoutes()); + app.use(errorHandler); + + return app; +} diff --git a/media-processor/src/index.ts b/media-processor/src/index.ts index 867fe33078..2d4e8c9260 100644 --- a/media-processor/src/index.ts +++ b/media-processor/src/index.ts @@ -1,15 +1,6 @@ -import express, { type Express } from "express"; import { loadConfig } from "./config.js"; import { initLogger, logger, logActiveTransports } from "./logger.js"; -import { healthRoutes } from "./routes/health.js"; -import { createImageRoutes } from "./routes/image.js"; -import { createFontRoutes } from "./routes/font.js"; -import { createSvgRoutes } from "./routes/svg.js"; -import { errorHandler } from "./middleware/error-handler.js"; -import { timeoutMiddleware } from "./middleware/timeout.js"; -import { sharedKeyAuth } from "./middleware/auth.js"; -import { createQueueMiddleware } from "./middleware/queue.js"; -import { loggingMiddleware } from "./middleware/logging.js"; +import { createApp } from "./app.js"; import { configureImageLimits } from "./services/image.js"; import { configureFontLimits } from "./services/font.js"; import { configureSvgLimits } from "./services/svg.js"; @@ -27,7 +18,6 @@ sharp.cache(false); const config = loadConfig(); initLogger(config); -const app: Express = express(); // Configure resource limits configureImageLimits({ @@ -52,16 +42,7 @@ configureSvgPool({ configureUploadLimits({ maxFileSize: config.maxFileSize, memoryThreshold: config.memoryThreshold }); -const queueMiddleware = createQueueMiddleware(config.maxConcurrentRequests); - -app.use(timeoutMiddleware(config.requestTimeout)); -app.use(loggingMiddleware); - -app.get("/api/health", healthRoutes); -app.use("/api/image", sharedKeyAuth(config.sharedKey), queueMiddleware, createImageRoutes()); -app.use("/api/font", sharedKeyAuth(config.sharedKey), queueMiddleware, createFontRoutes()); -app.use("/api/svg", sharedKeyAuth(config.sharedKey), queueMiddleware, createSvgRoutes()); -app.use(errorHandler); +const app = createApp(config); app.listen(config.port, config.host, () => { logActiveTransports(logger); diff --git a/media-processor/test/queue-isolation.test.ts b/media-processor/test/queue-isolation.test.ts new file mode 100644 index 0000000000..6a34f8750e --- /dev/null +++ b/media-processor/test/queue-isolation.test.ts @@ -0,0 +1,74 @@ +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import request from "supertest"; +import sharp from "sharp"; +import { EventEmitter } from "node:events"; +import { createApp } from "../src/app.js"; +import { configureUploadLimits } from "../src/upload.js"; +import { configureSvgPool, shutdownSvgPool, setSvgWorkerFactory, type SvgWorker } from "../src/svg-pool.js"; + +// A worker that never answers, so an SVG request parks on the pool while it +// holds its queue slot. +class HangingWorker extends EventEmitter { + public postMessage(): void {} + public async terminate(): Promise { + this.emit("exit", 0); + return 0; + } +} + +const config = { sharedKey: "test-key", maxConcurrentRequests: 1, requestTimeout: 10000 }; + +describe("queue isolation", () => { + let app: ReturnType; + + beforeAll(() => { + configureUploadLimits({ maxFileSize: 10 * 1024 * 1024, memoryThreshold: 10 }); + setSvgWorkerFactory(() => new HangingWorker() as unknown as SvgWorker); + configureSvgPool({ workers: 1, maxOldGenerationSizeMb: 512, timeout: 5000 }); + app = createApp(config); + }); + + afterAll(async () => { + await shutdownSvgPool(); + setSvgWorkerFactory(null); + }); + + it("a pending SVG request does not block an image request", async () => { + const svgRequest = request(app) + .post("/api/svg/sanitize") + .set("x-shared-key", "test-key") + .attach("file", Buffer.from(``), { + filename: "x.svg", + contentType: "image/svg+xml", + }) + .then((res) => res.status) + .catch(() => 0); + + // Let the SVG request be admitted and reach the hanging worker. + await new Promise((resolve) => setTimeout(resolve, 100)); + + const image = await sharp({ + create: { width: 10, height: 10, channels: 3, background: { r: 1, g: 2, b: 3 } }, + }) + .jpeg() + .toBuffer(); + + const imageRequest = request(app) + .post("/api/image/info") + .set("x-shared-key", "test-key") + .attach("file", image, { filename: "i.jpg", contentType: "image/jpeg" }) + .then((res) => res.status) + .catch(() => 0); + + const outcome = await Promise.race([ + imageRequest, + new Promise<"timeout">((resolve) => setTimeout(() => resolve("timeout"), 2000)), + ]); + + expect(outcome).toBe(200); + + // Release the held SVG request so the test ends clean. + await shutdownSvgPool(); + await svgRequest; + }); +});