From 0c9a8be05ed39455ea29cbf03e824539350c1eea Mon Sep 17 00:00:00 2001 From: Frank Leng <6145910+frankleng@users.noreply.github.com> Date: Wed, 7 Oct 2026 18:44:41 -0700 Subject: [PATCH] FileStore: write cache entries atomically FileStore.set rewrote each entry in place with fs.promises.writeFile, which truncates the file and then writes it in chunks. A concurrent reader of the same key could see a file whose head was still zeros, which get() returns as a binary (Buffer) entry, and the build then fails with "dependencies is not iterable". Write to a unique temporary file in the same directory and rename it into place, removing the temporary file if the write fails. The on-disk format is unchanged. --- packages/metro-cache/src/stores/FileStore.js | 15 +++- .../src/stores/__tests__/FileStore-test.js | 74 +++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/packages/metro-cache/src/stores/FileStore.js b/packages/metro-cache/src/stores/FileStore.js index 878898d743..43a872487a 100644 --- a/packages/metro-cache/src/stores/FileStore.js +++ b/packages/metro-cache/src/stores/FileStore.js @@ -9,6 +9,7 @@ * @oncall react_native */ +import crypto from 'node:crypto'; import fs from 'node:fs'; import path from 'node:path'; @@ -65,7 +66,19 @@ export default class FileStore { } else { content = JSON.stringify(value) ?? JSON.stringify(null); } - await fs.promises.writeFile(filePath, content); + // Write to a unique temporary file and rename it into place, so that a + // concurrent reader (or writer) of the same key never sees a partially + // written entry. Writing in place truncates the file first, and a reader + // could then see a run of null bytes, which looks like a binary entry. + const suffix = crypto.randomBytes(8).toString('hex'); + const tempPath = `${filePath}.${process.pid}.${suffix}.tmp`; + try { + await fs.promises.writeFile(tempPath, content); + await fs.promises.rename(tempPath, filePath); + } catch (err) { + await fs.promises.rm(tempPath, {force: true}); + throw err; + } } clear() { diff --git a/packages/metro-cache/src/stores/__tests__/FileStore-test.js b/packages/metro-cache/src/stores/__tests__/FileStore-test.js index e1377e673d..740ca51993 100644 --- a/packages/metro-cache/src/stores/__tests__/FileStore-test.js +++ b/packages/metro-cache/src/stores/__tests__/FileStore-test.js @@ -67,4 +67,78 @@ describe('FileStore', () => { await fileStore.set(cache, data); expect(await fileStore.get(cache)).toEqual(data); }); + + test('never exposes a partially written entry to a reader', async () => { + const fileStore = new FileStore({root: '/root'}); + const cache = Buffer.from([0xfa, 0xce, 0xb0, 0x0c]); + const previous = {dependencies: [], output: ['previous']}; + const next = {dependencies: [], output: ['x'.repeat(4096)]}; + await fileStore.set(cache, previous); + + // Simulate a write that is only part way done: the file has been opened + // (and truncated) and has zeros where its first half belongs. This is the + // state a concurrent writer of the same key can leave behind between the + // chunks of fs.promises.writeFile. + let resumeWrite: () => void = () => {}; + let onWritePaused: () => void = () => {}; + const writePaused = new Promise(resolve => { + onWritePaused = resolve; + }); + jest + .spyOn(fs.promises, 'writeFile') + .mockImplementationOnce(async (filePath, content) => { + const bytes = Buffer.from(content); + const half = Math.floor(bytes.length / 2); + const handle = await fs.promises.open(filePath, 'w'); + try { + await handle.write(Buffer.alloc(half), 0, half, 0); + await handle.write(bytes, half, bytes.length - half, half); + await new Promise(resolve => { + resumeWrite = resolve; + onWritePaused(); + }); + await handle.write(bytes, 0, half, 0); + } finally { + await handle.close(); + } + }); + + const pendingSet = fileStore.set(cache, next); + await writePaused; + const duringWrite = await fileStore.get(cache); + resumeWrite(); + await pendingSet; + + expect(duringWrite).toEqual(previous); + expect(await fileStore.get(cache)).toEqual(next); + }); + + test('leaves no temporary files behind', async () => { + const fileStore = new FileStore({root: '/root'}); + const cache = Buffer.from([0xfa, 0xce, 0xb0, 0x0c]); + + await Promise.all( + Array.from({length: 4}, (_, i) => fileStore.set(cache, {i})), + ); + + expect(fs.readdirSync('/root/fa')).toEqual(['ceb00c']); + }); + + test('removes the temporary file when a write fails', async () => { + const fileStore = new FileStore({root: '/root'}); + const cache = Buffer.from([0xfa, 0xce, 0xb0, 0x0c]); + await fileStore.set(cache, {foo: 42}); + const writeError = new Error('Disk full'); + + jest + .spyOn(fs.promises, 'writeFile') + .mockImplementationOnce(async (filePath, content) => { + await fs.promises.appendFile(filePath, content.slice(0, 1)); + throw writeError; + }); + + await expect(fileStore.set(cache, {foo: 43})).rejects.toBe(writeError); + expect(fs.readdirSync('/root/fa')).toEqual(['ceb00c']); + expect(await fileStore.get(cache)).toEqual({foo: 42}); + }); });