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 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 new file mode 100644 index 0000000..4618c55 --- /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.release(); + expect(live.size).toBe(0); + }); +});