From 1c6c9ccc1f5c6381e0d33956e63ba92e3976b536 Mon Sep 17 00:00:00 2001 From: ekklesia Date: Mon, 5 Oct 2026 18:36:47 -0500 Subject: [PATCH 1/3] test(display): one live object URL per cached slide, released when done The media cache minted a new object URL on every lookup and never revoked one. Each state refresh looks up every cached slide, so a display leaked one pinned blob per slide per refresh. In a real browser (Ekklesia draft PR #56, scenario stream-recycle) its own state requests then failed with net::ERR_INSUFFICIENT_RESOURCES within about four minutes, and it showed its offline banner on a working network. Fails today: 303 live URLs for 3 slides after 100 refreshes. The same bytes get a new URL each time, nothing is revoked on prune or replace, and there is no release. Co-Authored-By: Claude Opus 5.5 --- tests/browser/media-cache.test.ts | 119 ++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 tests/browser/media-cache.test.ts diff --git a/tests/browser/media-cache.test.ts b/tests/browser/media-cache.test.ts new file mode 100644 index 0000000..9754dbe --- /dev/null +++ b/tests/browser/media-cache.test.ts @@ -0,0 +1,119 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { createBrowserMediaCache, type SignageMediaCache } from "../../src/browser/storage.js"; + +/** + * The Cache Storage media cache hands out object URLs, and every live one pins its blob in the + * browser until it is revoked. + * + * Every state refresh looks up every cached slide, so a cache that minted a fresh URL per lookup + * leaked one blob per slide per refresh. In a real browser that exhausted blob storage within + * minutes: the display's own state requests then failed with `net::ERR_INSUFFICIENT_RESOURCES` and + * it fell back to its offline banner on a perfectly good network. + */ + +/** Just enough Cache Storage: one named cache, keyed by request URL. */ +function fakeCacheStorage() { + const entries = new Map(); + const cache = { + async delete(request: Request | string): Promise { + return entries.delete(typeof request === "string" ? request : request.url); + }, + async keys(): Promise { + return [...entries.keys()].map((url) => new Request(url)); + }, + async match(request: Request | string): Promise { + const entry = entries.get(typeof request === "string" ? request : request.url); + return entry + ? new Response(entry.body, { headers: { "content-type": entry.type } }) + : undefined; + }, + async put(request: Request | string, response: Response): Promise { + entries.set(typeof request === "string" ? request : request.url, { + body: await response.arrayBuffer(), + type: response.headers.get("content-type") ?? "application/octet-stream", + }); + }, + }; + return { open: async () => cache } as unknown as CacheStorage; +} + +describe("Signage's media cache and object URLs", () => { + const realCreate = URL.createObjectURL; + const realRevoke = URL.revokeObjectURL; + let live: Set; + let media: SignageMediaCache; + + beforeEach(() => { + live = new Set(); + let next = 0; + URL.createObjectURL = () => { + next += 1; + const url = `blob:test/${next}`; + live.add(url); + return url; + }; + URL.revokeObjectURL = (url: string) => { + live.delete(url); + }; + media = createBrowserMediaCache({ + caches: fakeCacheStorage(), + fetch: async () => + new Response(new Uint8Array([1, 2, 3]), { headers: { "content-type": "image/webp" } }), + location: { origin: "https://api.example.test" }, + } as unknown as typeof globalThis); + }); + + afterEach(() => { + URL.createObjectURL = realCreate; + URL.revokeObjectURL = realRevoke; + }); + + const keys = ["slide/a", "slide/b", "slide/c"]; + + it("keeps one live object URL per cached slide, however often it is looked up", async () => { + for (const key of keys) { + await media.store(key, `https://files.example.test/${key}?sig=1`); + } + // A state refresh looks every slide up again; a long-running display does this for months. + for (let refresh = 0; refresh < 100; refresh += 1) { + for (const key of keys) { + await media.find(key); + } + } + expect(live.size).toBe(keys.length); + }); + + it("hands out the same URL for the same bytes", async () => { + await media.store("slide/a", "https://files.example.test/a?sig=1"); + const first = await media.find("slide/a"); + const second = await media.find("slide/a"); + expect(first).toBeDefined(); + expect(second).toBe(first); + }); + + it("revokes a pruned slide's URL", async () => { + for (const key of keys) { + await media.store(key, `https://files.example.test/${key}?sig=1`); + await media.find(key); + } + await media.prune(["slide/a"]); + expect(live.size).toBe(1); + expect(await media.find("slide/b")).toBeUndefined(); + }); + + it("revokes the URL it replaces when a slide is stored again", async () => { + await media.store("slide/a", "https://files.example.test/a?sig=1"); + await media.store("slide/a", "https://files.example.test/a?sig=2"); + await media.find("slide/a"); + expect(live.size).toBe(1); + }); + + it("releases every URL it handed out when the display stops", async () => { + for (const key of keys) { + await media.store(key, `https://files.example.test/${key}?sig=1`); + await media.find(key); + } + (media as SignageMediaCache & { release(): void }).release(); + expect(live.size).toBe(0); + }); +}); -- 2.54.0 From 607f37580935ed80096a4511c457fd3de5fd5459 Mon Sep 17 00:00:00 2001 From: ekklesia Date: Mon, 5 Oct 2026 18:37:35 -0500 Subject: [PATCH 2/3] fix(display): keep one object URL per cached slide, and give them back The Cache Storage media cache minted a new object URL on every lookup and never revoked any. Every state refresh looks up every cached slide, so a running display pinned one more blob per slide per refresh until the browser refused its requests with net::ERR_INSUFFICIENT_RESOURCES. It then showed its offline banner on a working network. Two tabs leaked twice as fast, and real photographs fill blob storage far sooner than test slides. Each cache entry now has exactly one object URL: - every lookup reuses it; - pruning or replacing the entry revokes it; - release(), new on SignageMediaCache, revokes all of them, and the controller calls it on stop once the stage is gone. Two lookups of one slide racing each other keep the first URL and make no second one. The in-memory cache has nothing to give back. Co-Authored-By: Claude Opus 5.5 --- src/browser/storage.ts | 52 +++++++++++++++++++++++++++++-- src/display/controller.ts | 2 ++ tests/browser/media-cache.test.ts | 2 +- 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/browser/storage.ts b/src/browser/storage.ts index ca883c1..06c6867 100644 --- a/src/browser/storage.ts +++ b/src/browser/storage.ts @@ -36,6 +36,9 @@ export interface SignageMediaCache { * can never leave a screen with less than it had. */ prune(keep: readonly string[]): Promise; count(): Promise; + /** Gives back every local URL this cache handed out. Called when a display stops; a URL handed + * out before this is no longer valid afterwards. */ + release(): void; } /** The stable key a native image's bytes are cached under. Deliberately built from identity, never @@ -86,6 +89,9 @@ export function createMemoryMediaCache(): SignageMediaCache { } return Promise.resolve(); }, + release(): void { + // Holds plain URLs, nothing the browser has to be given back. + }, store(key: string, url: string): Promise { stored.set(key, url); return Promise.resolve(url); @@ -179,14 +185,45 @@ export function createBrowserMediaCache(scope: typeof globalThis = globalThis): return `${origin}/${key}`; } - async function localUrl(cache: Cache, url: string): Promise { - const response = await cache.match(url); + /** + * The one object URL each cached slide is rendered from, by cache entry. + * + * An object URL pins its blob in the browser until it is revoked. Every state refresh looks every + * cached slide up again, so minting a URL per lookup leaked a blob per slide per refresh, and + * within minutes the browser refused the display's own requests for want of resources. So each + * entry gets exactly one URL, revoked when the entry is pruned or replaced, and all of them when + * the display stops. + */ + const objectUrls = new Map(); + + function forget(request: string): void { + const url = objectUrls.get(request); + if (url !== undefined) { + objectUrls.delete(request); + URL.revokeObjectURL(url); + } + } + + async function localUrl(cache: Cache, request: string): Promise { + const existing = objectUrls.get(request); + if (existing !== undefined) { + return existing; + } + const response = await cache.match(request); if (!response) { return undefined; } try { const blob = await response.blob(); - return URL.createObjectURL(blob); + // Two lookups of one slide can be in flight at once; the first to finish wins, and the other + // gives its URL straight back rather than leaving it pinned. + const raced = objectUrls.get(request); + if (raced !== undefined) { + return raced; + } + const url = URL.createObjectURL(blob); + objectUrls.set(request, url); + return url; } catch { return undefined; } @@ -220,6 +257,7 @@ export function createBrowserMediaCache(scope: typeof globalThis = globalThis): for (const request of await cache.keys()) { if (!retained.has(request.url)) { await cache.delete(request); + forget(request.url); } } } catch { @@ -227,6 +265,12 @@ export function createBrowserMediaCache(scope: typeof globalThis = globalThis): } }, + release(): void { + for (const request of [...objectUrls.keys()]) { + forget(request); + } + }, + async store(key: string, url: string): Promise { const cache = await open(); if (!cache) { @@ -242,6 +286,8 @@ export function createBrowserMediaCache(scope: typeof globalThis = globalThis): // Stored under the *stable* key, so the next refresh's freshly signed URL finds these // bytes instead of downloading them again. await cache.put(cacheRequest(key), response.clone()); + // New bytes under this key: the URL for the old ones is given back before one is made. + forget(cacheRequest(key)); return localUrl(cache, cacheRequest(key)); } catch { return undefined; diff --git a/src/display/controller.ts b/src/display/controller.ts index d531c11..2cdf6d3 100644 --- a/src/display/controller.ts +++ b/src/display/controller.ts @@ -715,6 +715,8 @@ export function createDisplayController(options: DisplayControllerOptions): Disp exitOffline(); await stage?.destroy(); stage = undefined; + // After the stage is gone, so nothing on screen still points at a released URL. + media.release(); }, }; diff --git a/tests/browser/media-cache.test.ts b/tests/browser/media-cache.test.ts index 9754dbe..4618c55 100644 --- a/tests/browser/media-cache.test.ts +++ b/tests/browser/media-cache.test.ts @@ -113,7 +113,7 @@ describe("Signage's media cache and object URLs", () => { await media.store(key, `https://files.example.test/${key}?sig=1`); await media.find(key); } - (media as SignageMediaCache & { release(): void }).release(); + media.release(); expect(live.size).toBe(0); }); }); -- 2.54.0 From e3c5e546eb795dd97f1c504f68c34bf2edd06120 Mon Sep 17 00:00:00 2001 From: ekklesia Date: Mon, 5 Oct 2026 21:45:28 -0500 Subject: [PATCH 3/3] ci: run stream-recycle on every change It failed on main before this PR: the media cache leaked one blob per slide per refresh until the browser refused the display's own requests. With one object URL per slide it holds three live URLs for three slides, and CI now keeps it there, together with "stays live for the whole watch". Co-Authored-By: Claude Opus 5.5 --- ci/display-scenarios | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/display-scenarios b/ci/display-scenarios index 1d06710..06ee010 100644 --- a/ci/display-scenarios +++ b/ci/display-scenarios @@ -4,3 +4,4 @@ two-pages session-blip stream-failure +stream-recycle -- 2.54.0