diff --git a/.changeset/tidy-writers-publish.md b/.changeset/tidy-writers-publish.md new file mode 100644 index 000000000000..88697a6e4fb8 --- /dev/null +++ b/.changeset/tidy-writers-publish.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: atomically replace generated files during concurrent syncs diff --git a/packages/kit/src/core/sync/utils.js b/packages/kit/src/core/sync/utils.js index 8faa1ad09582..054b94fcea26 100644 --- a/packages/kit/src/core/sync/utils.js +++ b/packages/kit/src/core/sync/utils.js @@ -1,5 +1,6 @@ import fs from 'node:fs'; import path from 'node:path'; +import { randomUUID } from 'node:crypto'; import { resolve_entry } from '../../utils/filesystem.js'; import * as w from '../../messages/build-warnings.js'; @@ -23,7 +24,16 @@ export function write_if_changed(file, code) { */ export function write(file, code) { fs.mkdirSync(path.dirname(file), { recursive: true }); - fs.writeFileSync(file, code); + // Concurrent syncs can write different contents (e.g. when outDir differs). Publish + // a complete file so neither readers nor other writers see a truncated destination. + // Avoid extending the destination basename, which may already be near filesystem limits. + const temporary = path.join(path.dirname(file), `${randomUUID()}.tmp`); + try { + fs.writeFileSync(temporary, code); + fs.renameSync(temporary, file); + } finally { + fs.rmSync(temporary, { force: true }); + } } /** diff --git a/packages/kit/src/core/sync/utils.spec.js b/packages/kit/src/core/sync/utils.spec.js index 3528dccc6f72..a30ba210842c 100644 --- a/packages/kit/src/core/sync/utils.spec.js +++ b/packages/kit/src/core/sync/utils.spec.js @@ -3,7 +3,7 @@ import os from 'node:os'; import path from 'node:path'; import process from 'node:process'; import { afterAll, expect, test, vi } from 'vitest'; -import { check_spelling, write_if_changed } from './utils.js'; +import { check_spelling, write, write_if_changed } from './utils.js'; const fixtures = path.join(import.meta.dirname, 'fixtures'); @@ -52,6 +52,112 @@ test.describe('write', () => { fs.rmSync(dir, { recursive: true, force: true }); }); + test('writes files with basenames near the filesystem length limit', () => { + const directory = path.join(dir, 'long-filename'); + const basename = `${'a'.repeat(230)}.html`; + const file = path.join(directory, basename); + + write(file, 'before'); + write(file, 'after'); + + expect(fs.readFileSync(file, 'utf-8')).toBe('after'); + expect(fs.readdirSync(directory)).toEqual([basename]); + }); + + test('concurrent writes of different lengths do not leave trailing bytes', () => { + const file = path.join(dir, 'concurrent.json'); + const shorter = JSON.stringify({ rootDirs: ['.svelte-kit-a/types'] }); + const longer = JSON.stringify({ rootDirs: ['.svelte-kit-ab/types'] }); + const write_file = fs.writeFileSync; + let interleaved = false; + + // Reproduce the OS interleaving deterministically: both writers open/truncate + // their destination before the longer write finishes, then the shorter one wins. + const spy = vi.spyOn(fs, 'writeFileSync').mockImplementation((target, code, options) => { + const fd = fs.openSync(/** @type {string} */ (target), 'w'); + try { + if (!interleaved) { + interleaved = true; + write_if_changed(file, longer); + } + write_file(fd, code, options); + } finally { + fs.closeSync(fd); + } + }); + + try { + write_if_changed(file, shorter); + } finally { + spy.mockRestore(); + } + + expect(fs.readFileSync(file, 'utf-8')).toBe(shorter); + expect(JSON.parse(fs.readFileSync(file, 'utf-8'))).toEqual(JSON.parse(shorter)); + expect(fs.readdirSync(dir).filter((name) => name.endsWith('.tmp'))).toEqual([]); + }); + + test('readers see the previous contents until the replacement is complete', () => { + const file = path.join(dir, 'reader.js'); + write(file, 'before'); + + const write_file = fs.writeFileSync; + const spy = vi.spyOn(fs, 'writeFileSync').mockImplementationOnce((target, code, options) => { + write_file(target, code, options); + expect(fs.readFileSync(file, 'utf-8')).toBe('before'); + }); + + try { + write(file, 'after'); + } finally { + spy.mockRestore(); + } + + expect(fs.readFileSync(file, 'utf-8')).toBe('after'); + }); + + test('a failed temporary write preserves the destination and removes partial output', () => { + const directory = path.join(dir, 'failed-write'); + const file = path.join(directory, 'generated.js'); + write(file, 'before'); + + const error = new Error('write failed'); + const write_file = fs.writeFileSync; + const spy = vi.spyOn(fs, 'writeFileSync').mockImplementationOnce((target) => { + write_file(target, 'partial'); + throw error; + }); + + try { + expect(() => write(file, 'after')).toThrow(error); + } finally { + spy.mockRestore(); + } + + expect(fs.readFileSync(file, 'utf-8')).toBe('before'); + expect(fs.readdirSync(directory)).toEqual(['generated.js']); + }); + + test('a failed rename preserves the destination and removes temporary output', () => { + const directory = path.join(dir, 'failed-rename'); + const file = path.join(directory, 'generated.js'); + write(file, 'before'); + + const error = Object.assign(new Error('rename failed'), { code: 'EACCES' }); + const spy = vi.spyOn(fs, 'renameSync').mockImplementationOnce(() => { + throw error; + }); + + try { + expect(() => write(file, 'after')).toThrow(error); + } finally { + spy.mockRestore(); + } + + expect(fs.readFileSync(file, 'utf-8')).toBe('before'); + expect(fs.readdirSync(directory)).toEqual(['generated.js']); + }); + test('write_if_changed leaves a file that another process already wrote alone', () => { const file = path.join(dir, 'unchanged.js');