Skip to content

Commit 241cec5

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 734bb76 commit 241cec5

2 files changed

Lines changed: 35 additions & 21 deletions

File tree

lib/copy-directory.js

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ const { promises: fs } = require('graceful-fs')
44
const crypto = require('crypto')
55
const path = require('path')
66

7-
const { backOff } = require('exponential-backoff')
7+
const RACE_ERRORS = ['ENOTEMPTY', 'EEXIST', 'EBUSY', 'EPERM']
88

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

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)