Skip to content

Commit d89ac9f

Browse files
authored
feat(core): avoid race conditions when writing temp files (#1674)
1 parent 3d65ecd commit d89ac9f

File tree

2 files changed

+146
-2
lines changed

2 files changed

+146
-2
lines changed
Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,53 @@
1-
import { fs } from '@vuepress/utils'
1+
import { fs, hash } from '@vuepress/utils'
22
import type { AppDir, AppWriteTemp } from '../types/index.js'
33

4+
interface WriteTempCache {
5+
hash?: string // content hash
6+
current?: Promise<void> // the current writing promise
7+
next?: () => Promise<void> // the next writing promise
8+
}
49
/**
510
* Resolve write temp file util for vuepress app
611
*
712
* @internal
813
*/
914
export const resolveAppWriteTemp = (dir: AppDir): AppWriteTemp => {
15+
const cache = new Map<string, WriteTempCache>()
16+
1017
const writeTemp: AppWriteTemp = async (file: string, content: string) => {
1118
const filePath = dir.temp(file)
12-
await fs.outputFile(filePath, content)
19+
const contentHash = hash(content)
20+
21+
let item = cache.get(filePath)
22+
if (!item) {
23+
cache.set(filePath, (item = {}))
24+
}
25+
26+
// if content hash is the same as the last one, skip writing
27+
if (item.hash === contentHash) {
28+
return filePath
29+
}
30+
31+
item.hash = contentHash
32+
33+
if (!item.current) {
34+
item.current = (async () => {
35+
await fs.outputFile(filePath, content)
36+
// if there is a next writing promise, chain it with the current one
37+
item.current = item.next?.()
38+
return item.current
39+
})()
40+
} else {
41+
// if there is a current writing promise, save the next writing promise
42+
item.next = async () => {
43+
await fs.outputFile(filePath, content)
44+
item.next = undefined
45+
item.current = undefined
46+
}
47+
}
48+
await item.current
1349
return filePath
1450
}
51+
1552
return writeTemp
1653
}
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
import { fs } from '@vuepress/utils'
2+
import { describe, expect, it, vi } from 'vitest'
3+
import { createAppDirFunction } from '../../src/app/resolveAppDir.js'
4+
import { resolveAppWriteTemp } from '../../src/app/resolveAppWriteTemp.js'
5+
import type { AppDir } from '../../src/index.js'
6+
7+
vi.mock('@vuepress/utils', async (importOriginal) => {
8+
// eslint-disable-next-line @typescript-eslint/consistent-type-imports
9+
const actual = await importOriginal<typeof import('@vuepress/utils')>()
10+
return {
11+
...actual,
12+
fs: {
13+
...actual.fs,
14+
outputFile: vi.fn(),
15+
},
16+
}
17+
})
18+
19+
describe('resolveAppWriteTemp', () => {
20+
const dir = {
21+
temp: createAppDirFunction('/temp'),
22+
} as AppDir
23+
24+
it('should write temp file correctly', async () => {
25+
const writeTemp = resolveAppWriteTemp(dir)
26+
const file = 'foo.txt'
27+
const content = 'bar'
28+
await writeTemp(file, content)
29+
expect(fs.outputFile).toHaveBeenCalledWith(dir.temp(file), 'bar')
30+
})
31+
32+
it('should avoid overwriting newer content with older content (race condition)', async () => {
33+
const writeTemp = resolveAppWriteTemp(dir)
34+
const file = 'race.txt'
35+
const log: string[] = []
36+
37+
// eslint-disable-next-line @typescript-eslint/no-misused-promises
38+
vi.mocked(fs.outputFile).mockImplementation(async (_f, c) => {
39+
const content = c as string
40+
// Simulate delay: '1' is slow, '2' is fast
41+
const delay = content === '1' ? 50 : 10
42+
await new Promise((resolve) => {
43+
setTimeout(resolve, delay)
44+
})
45+
log.push(content)
46+
})
47+
48+
// Call '1' then '2' immediately
49+
const p1 = writeTemp(file, '1')
50+
const p2 = writeTemp(file, '2')
51+
52+
await Promise.all([p1, p2])
53+
54+
// With parallel execution and delays, '2' finishes first, then '1' overwrites it.
55+
// We expect the fix to ensure '1' finishes then '2', or '2' is the final write.
56+
// Ideally sequential execution: '1' writes, then '2' writes.
57+
expect(log).toEqual(['1', '2'])
58+
})
59+
60+
it('should skip write if content is unchanged', async () => {
61+
vi.mocked(fs.outputFile).mockClear()
62+
const writeTemp = resolveAppWriteTemp(dir)
63+
const file = 'cache.txt'
64+
const content = 'foo'
65+
66+
await writeTemp(file, content)
67+
expect(fs.outputFile).toHaveBeenCalledTimes(1)
68+
69+
await writeTemp(file, content)
70+
expect(fs.outputFile).toHaveBeenCalledTimes(1)
71+
72+
await writeTemp(file, 'bar')
73+
expect(fs.outputFile).toHaveBeenCalledTimes(2)
74+
})
75+
76+
it('should skip intermediate writes', async () => {
77+
vi.mocked(fs.outputFile).mockClear()
78+
const writeTemp = resolveAppWriteTemp(dir)
79+
const file = 'skip.txt'
80+
const log: string[] = []
81+
82+
// eslint-disable-next-line @typescript-eslint/no-misused-promises
83+
vi.mocked(fs.outputFile).mockImplementation(async (_f, c) => {
84+
const content = c as string
85+
// Simulate delay to ensure queuing
86+
await new Promise((resolve) => {
87+
setTimeout(resolve, 20)
88+
})
89+
log.push(content)
90+
})
91+
92+
// 1. First write starts immediately
93+
const p1 = writeTemp(file, '1')
94+
95+
// 2. Second and Third write come in while First is running
96+
// They should be collapsed into a single subsequent write
97+
const p2 = writeTemp(file, '2')
98+
const p3 = writeTemp(file, '3')
99+
100+
await Promise.all([p1, p2, p3])
101+
102+
// '1' is written (started before others).
103+
// '2' is skipped because '3' arrived while '1' was running, updating the nextContent.
104+
// '3' is written after '1' finishes.
105+
expect(log).toEqual(['1', '3'])
106+
})
107+
})

0 commit comments

Comments
 (0)