Skip to content

Commit 0a46dac

Browse files
committed
fix: ensure copyDirectory atomically copies files
Assisted-by: Claude Opus 4.6 Signed-off-by: David Sanders <dsanders11@ucsbalum.com>
1 parent c2d7674 commit 0a46dac

2 files changed

Lines changed: 35 additions & 22 deletions

File tree

lib/copy-directory.js

Lines changed: 35 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@
33
const { promises: fs } = require('graceful-fs')
44
const path = require('path')
55

6-
const { backOff } = require('exponential-backoff')
7-
8-
async function copyDirectory (src, dest, ensure = false) {
6+
async function copyDirectory (src, dest) {
97
try {
108
await fs.stat(src)
119
} catch {
@@ -14,27 +12,43 @@ async function copyDirectory (src, dest, ensure = false) {
1412
await fs.mkdir(dest, { recursive: true })
1513
const entries = await fs.readdir(src, { withFileTypes: true })
1614
for (const entry of entries) {
17-
if (entry.isDirectory()) {
18-
await copyDirectory(path.join(src, entry.name), path.join(dest, entry.name))
19-
} else if (entry.isFile()) {
20-
// with parallel installs, copying files may cause file errors on
21-
// Windows so use an exponential backoff to resolve collisions
22-
await backOff(async () => {
15+
if (!entry.isDirectory() && !entry.isFile()) {
16+
throw new Error('Unexpected file directory entry type')
17+
}
18+
19+
// With parallel installs, multiple processes race to place the same
20+
// entry. Use fs.rename for an atomic move so no process ever sees a
21+
// partially written file. For cross-filesystem (EXDEV), copy to a
22+
// temp path in the dest directory first, then rename within the
23+
// same filesystem to keep it atomic.
24+
//
25+
// When another process wins the race, rename may fail with one of
26+
// these codes — all mean the destination was already placed and
27+
// are safe to ignore since every process extracts identical content.
28+
const raceErrors = ['ENOTEMPTY', 'EEXIST', 'EBUSY', 'EPERM']
29+
const srcPath = path.join(src, entry.name)
30+
const destPath = path.join(dest, entry.name)
31+
try {
32+
await fs.rename(srcPath, destPath)
33+
} catch (err) {
34+
if (raceErrors.includes(err.code)) {
35+
// Another parallel process already placed this entry — ignore
36+
} else if (err.code === 'EXDEV') {
37+
// Cross-filesystem: copy to a uniquely named temp path in the
38+
// dest directory, then rename into place atomically
39+
const tmpPath = `${destPath}.tmp.${crypto.randomBytes(6).toString('hex')}`
2340
try {
24-
await fs.copyFile(path.join(src, entry.name), path.join(dest, entry.name))
25-
} catch (err) {
26-
// if ensure, check if file already exists and that's good enough
27-
if (ensure && err.code === 'EBUSY') {
28-
try {
29-
await fs.stat(path.join(dest, entry.name))
30-
return
31-
} catch {}
41+
await fs.cp(srcPath, tmpPath, { recursive: true })
42+
await fs.rename(tmpPath, destPath)
43+
} catch (e) {
44+
await fs.rm(tmpPath, { recursive: true, force: true }).catch(() => {})
45+
if (!raceErrors.includes(e.code)) {
46+
throw e
3247
}
33-
throw err
3448
}
35-
})
36-
} else {
37-
throw new Error('Unexpected file directory entry type')
49+
} else {
50+
throw err
51+
}
3852
}
3953
}
4054
}

package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
"main": "./lib/node-gyp.js",
2424
"dependencies": {
2525
"env-paths": "^2.2.0",
26-
"exponential-backoff": "^3.1.1",
2726
"graceful-fs": "^4.2.6",
2827
"make-fetch-happen": "^15.0.0",
2928
"nopt": "^9.0.0",

0 commit comments

Comments
 (0)