From 2d9a1f969012f35e23b2bd8cdd4b509591969164 Mon Sep 17 00:00:00 2001 From: Steven Date: Tue, 1 Feb 2022 17:27:39 -0500 Subject: [PATCH 1/4] Fix image cache race condition --- packages/next/server/image-optimizer.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/next/server/image-optimizer.ts b/packages/next/server/image-optimizer.ts index 0d603d546ab58..cd3e0520e8a89 100644 --- a/packages/next/server/image-optimizer.ts +++ b/packages/next/server/image-optimizer.ts @@ -219,7 +219,6 @@ export async function imageOptimizer( if (isFresh) { return { finished: true } } else { - await promises.unlink(fsPath) staleWhileRevalidate = true } } From 0315ad68f96adbe58490e42c6b3856df97d643c6 Mon Sep 17 00:00:00 2001 From: Steven Date: Tue, 1 Feb 2022 21:49:26 -0500 Subject: [PATCH 2/4] Refactor xCache header and Deferred --- packages/next/server/image-optimizer.ts | 72 ++++++++++++++++--------- 1 file changed, 47 insertions(+), 25 deletions(-) diff --git a/packages/next/server/image-optimizer.ts b/packages/next/server/image-optimizer.ts index cd3e0520e8a89..dea85ecf5a6f8 100644 --- a/packages/next/server/image-optimizer.ts +++ b/packages/next/server/image-optimizer.ts @@ -19,6 +19,8 @@ import { getContentType, getExtension } from './serve-static' import chalk from 'next/dist/compiled/chalk' import { NextUrlWithParsedQuery } from './request-meta' +type XCacheHeader = 'MISS' | 'HIT' | 'STALE' + const AVIF = 'image/avif' const WEBP = 'image/webp' const PNG = 'image/png' @@ -29,7 +31,7 @@ const CACHE_VERSION = 3 const ANIMATABLE_TYPES = [WEBP, PNG, GIF] const VECTOR_TYPES = [SVG] const BLUR_IMG_SIZE = 8 // should match `next-image-loader` -const inflightRequests = new Map>() +const inflightRequests = new Map>() let sharp: | (( @@ -178,18 +180,16 @@ export async function imageOptimizer( const imagesDir = join(distDir, 'cache', 'images') const hashDir = join(imagesDir, hash) const now = Date.now() - let staleWhileRevalidate = false + const stale = new Deferred() + let xCache: XCacheHeader = 'MISS' // If there're concurrent requests hitting the same resource and it's still // being optimized, wait before accessing the cache. if (inflightRequests.has(hash)) { await inflightRequests.get(hash) } - let dedupeResolver: (val?: PromiseLike) => void - inflightRequests.set( - hash, - new Promise((resolve) => (dedupeResolver = resolve)) - ) + const dedupe = new Deferred() + inflightRequests.set(hash, dedupe.promise) try { if (await fileExists(hashDir, 'directory')) { @@ -200,8 +200,7 @@ export async function imageOptimizer( const expireAt = Number(expireAtSt) const contentType = getContentType(extension) const fsPath = join(hashDir, file) - const isFresh = now < expireAt - const xCache = isFresh ? 'HIT' : 'STALE' + xCache = now < expireAt ? 'HIT' : 'STALE' const result = setResponseHeaders( req, res, @@ -214,16 +213,21 @@ export async function imageOptimizer( xCache ) if (!result.finished) { - createReadStream(fsPath).pipe(res) + createReadStream(fsPath) + .on('end', () => stale.resolve(fsPath)) + .pipe(res) } - if (isFresh) { + if (xCache === 'HIT') { + stale.resolve(null) return { finished: true } - } else { - staleWhileRevalidate = true } } } + if (xCache === 'MISS') { + stale.resolve(null) + } + let upstreamBuffer: Buffer let upstreamType: string | null let maxAge: number @@ -326,7 +330,8 @@ export async function imageOptimizer( upstreamType, maxAge, expireAt, - upstreamBuffer + upstreamBuffer, + stale ) sendResponse( req, @@ -337,7 +342,7 @@ export async function imageOptimizer( upstreamBuffer, isStatic, isDev, - staleWhileRevalidate + xCache ) return { finished: true } } @@ -480,7 +485,8 @@ export async function imageOptimizer( contentType, maxAge, expireAt, - optimizedBuffer + optimizedBuffer, + stale ) sendResponse( req, @@ -491,7 +497,7 @@ export async function imageOptimizer( optimizedBuffer, isStatic, isDev, - staleWhileRevalidate + xCache ) } else { throw new Error('Unable to optimize buffer') @@ -506,14 +512,13 @@ export async function imageOptimizer( upstreamBuffer, isStatic, isDev, - staleWhileRevalidate + xCache ) } return { finished: true } } finally { - // Make sure to remove the hash in the end. - dedupeResolver!() + dedupe.resolve() inflightRequests.delete(hash) } } @@ -523,8 +528,13 @@ async function writeToCacheDir( contentType: string, maxAge: number, expireAt: number, - buffer: Buffer + buffer: Buffer, + stale: Deferred ) { + const expiredPath = await stale.promise + if (expiredPath) { + await promises.unlink(expiredPath) + } await promises.mkdir(dir, { recursive: true }) const extension = getExtension(contentType) const etag = getHash([buffer]) @@ -556,7 +566,7 @@ function setResponseHeaders( contentType: string | null, isStatic: boolean, isDev: boolean, - xCache: 'MISS' | 'HIT' | 'STALE' + xCache: XCacheHeader ) { res.setHeader('Vary', 'Accept') res.setHeader( @@ -596,12 +606,11 @@ function sendResponse( buffer: Buffer, isStatic: boolean, isDev: boolean, - staleWhileRevalidate: boolean + xCache: XCacheHeader ) { - if (staleWhileRevalidate) { + if (xCache === 'STALE') { return } - const xCache = 'MISS' const etag = getHash([buffer]) const result = setResponseHeaders( req, @@ -781,3 +790,16 @@ export async function getImageSize( const { width, height } = imageSizeOf(buffer) return { width, height } } + +export class Deferred { + promise: Promise + resolve!: (value: T) => void + reject!: (error?: Error) => void + + constructor() { + this.promise = new Promise((resolve, reject) => { + this.resolve = resolve + this.reject = reject + }) + } +} From 0ec5e51f4783c2a645bd695c929f96b3b083adf9 Mon Sep 17 00:00:00 2001 From: Steven Date: Wed, 2 Feb 2022 11:33:03 -0500 Subject: [PATCH 3/4] Remove stale deferred in favor of awaiting stream promise --- packages/next/server/image-optimizer.ts | 30 +++++++++---------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/packages/next/server/image-optimizer.ts b/packages/next/server/image-optimizer.ts index dea85ecf5a6f8..083cd1a447a5b 100644 --- a/packages/next/server/image-optimizer.ts +++ b/packages/next/server/image-optimizer.ts @@ -180,7 +180,6 @@ export async function imageOptimizer( const imagesDir = join(distDir, 'cache', 'images') const hashDir = join(imagesDir, hash) const now = Date.now() - const stale = new Deferred() let xCache: XCacheHeader = 'MISS' // If there're concurrent requests hitting the same resource and it's still @@ -213,21 +212,21 @@ export async function imageOptimizer( xCache ) if (!result.finished) { - createReadStream(fsPath) - .on('end', () => stale.resolve(fsPath)) - .pipe(res) + await new Promise((resolve, reject) => { + createReadStream(fsPath) + .on('end', () => resolve) + .on('error', (err) => reject(err)) + .pipe(res) + }) } if (xCache === 'HIT') { - stale.resolve(null) return { finished: true } + } else { + await promises.unlink(fsPath) } } } - if (xCache === 'MISS') { - stale.resolve(null) - } - let upstreamBuffer: Buffer let upstreamType: string | null let maxAge: number @@ -330,8 +329,7 @@ export async function imageOptimizer( upstreamType, maxAge, expireAt, - upstreamBuffer, - stale + upstreamBuffer ) sendResponse( req, @@ -485,8 +483,7 @@ export async function imageOptimizer( contentType, maxAge, expireAt, - optimizedBuffer, - stale + optimizedBuffer ) sendResponse( req, @@ -528,13 +525,8 @@ async function writeToCacheDir( contentType: string, maxAge: number, expireAt: number, - buffer: Buffer, - stale: Deferred + buffer: Buffer ) { - const expiredPath = await stale.promise - if (expiredPath) { - await promises.unlink(expiredPath) - } await promises.mkdir(dir, { recursive: true }) const extension = getExtension(contentType) const etag = getHash([buffer]) From c2beb8480e5c5f2e32ef96074895e875dfb3811a Mon Sep 17 00:00:00 2001 From: Steven Date: Wed, 2 Feb 2022 12:02:39 -0500 Subject: [PATCH 4/4] Fix typo --- packages/next/server/image-optimizer.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/next/server/image-optimizer.ts b/packages/next/server/image-optimizer.ts index 083cd1a447a5b..8363070892c60 100644 --- a/packages/next/server/image-optimizer.ts +++ b/packages/next/server/image-optimizer.ts @@ -212,10 +212,10 @@ export async function imageOptimizer( xCache ) if (!result.finished) { - await new Promise((resolve, reject) => { + await new Promise((resolve, reject) => { createReadStream(fsPath) - .on('end', () => resolve) - .on('error', (err) => reject(err)) + .on('end', resolve) + .on('error', reject) .pipe(res) }) }