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}); + }); });