Skip to content

Commit a6d54f2

Browse files
authored
fix(fileLock): improve lock acquisition reliability on Windows (#146)
- Add retry-based helper for lock acquisition to mitigate Windows race conditions - Improve stale lock handling with additional checks and small delays - Update tests with higher lock acquisition timeout for parallel runs
1 parent b76fc66 commit a6d54f2

File tree

2 files changed

+80
-8
lines changed

2 files changed

+80
-8
lines changed

src/utils/fileLock.ts

Lines changed: 77 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Uses exclusive file creation with PID tracking for stale lock detection
44
*/
55

6-
import { open, unlink, readFile } from 'node:fs/promises';
6+
import { open, unlink, readFile, access } from 'node:fs/promises';
77
import { constants } from 'node:fs';
88

99
export interface LockOptions {
@@ -151,6 +151,42 @@ export async function acquireLock(
151151
timestamp: Date.now(),
152152
};
153153

154+
// Helper function to try acquiring lock immediately with retries (for Windows race condition)
155+
const tryAcquireImmediately = async (): Promise<LockRelease | null> => {
156+
for (let retry = 0; retry < 5; retry++) {
157+
// Check timeout before each retry
158+
if (Date.now() - startTime >= timeout) {
159+
return null;
160+
}
161+
await new Promise(resolve => setTimeout(resolve, 10 + retry * 5)); // 10, 15, 20, 25, 30ms
162+
try {
163+
const handle = await open(lockPath, constants.O_CREAT | constants.O_EXCL | constants.O_WRONLY);
164+
await handle.writeFile(JSON.stringify(lockContent));
165+
await handle.close();
166+
// Successfully acquired lock!
167+
return {
168+
release: async () => {
169+
if (released) return;
170+
released = true;
171+
try {
172+
await unlink(lockPath);
173+
} catch {
174+
// Ignore errors during release
175+
}
176+
},
177+
};
178+
} catch (retryErr) {
179+
const retryError = retryErr as NodeJS.ErrnoException;
180+
if (retryError.code !== 'EEXIST') {
181+
// Not a file-exists error, something else went wrong - abort retry loop
182+
return null;
183+
}
184+
// Still exists, continue retrying
185+
}
186+
}
187+
return null; // Retry loop exhausted
188+
};
189+
154190
while (Date.now() - startTime < timeout) {
155191
try {
156192
// Try to create lock file exclusively (fails if exists)
@@ -175,18 +211,54 @@ export async function acquireLock(
175211

176212
if (err.code === 'EEXIST') {
177213
// Lock file exists - check if it's stale
178-
if (await isLockStale(lockPath, staleThreshold)) {
214+
const stale = await isLockStale(lockPath, staleThreshold);
215+
if (stale) {
179216
await removeStale(lockPath);
180-
// Small delay after removing stale lock to let filesystem catch up
217+
// Delay after removing stale lock to let filesystem catch up
181218
// Especially important on Windows where file deletion can be asynchronous
182-
await new Promise(resolve => setTimeout(resolve, 10));
219+
// Use longer delay under load (when tests run in parallel)
220+
await new Promise(resolve => setTimeout(resolve, 20));
183221
// Retry immediately after removing stale lock
184222
continue;
185223
}
186224

187225
// Lock is held by another active process - wait and retry
226+
// On Windows, file deletion can be asynchronous, so we need to be more careful.
227+
// Check if file still exists before waiting - if it's already gone, try to acquire immediately.
228+
try {
229+
await access(lockPath);
230+
} catch {
231+
// File no longer exists - try to acquire immediately with retries
232+
// On Windows, there's a race where access() says file is gone but open() still sees it.
233+
// Retry a few times with small delays to handle Windows filesystem propagation delays.
234+
const immediateResult = await tryAcquireImmediately();
235+
if (immediateResult) {
236+
return immediateResult;
237+
}
238+
// Retry loop exhausted, continue to normal wait logic
239+
continue;
240+
}
241+
242+
// File exists, wait before retrying
188243
await new Promise(resolve => setTimeout(resolve, retryInterval));
189-
continue;
244+
245+
// After waiting, check again if file still exists
246+
// On Windows, the file might have been deleted while we waited, but the
247+
// filesystem hasn't fully propagated the deletion yet. Try to acquire immediately.
248+
try {
249+
await access(lockPath);
250+
// File still exists, continue to next iteration (will check stale again)
251+
continue;
252+
} catch {
253+
// File no longer exists - try to acquire immediately with retries
254+
// Same retry logic as above to handle Windows race condition
255+
const immediateResult = await tryAcquireImmediately();
256+
if (immediateResult) {
257+
return immediateResult;
258+
}
259+
// Retry loop exhausted, continue to normal wait/retry cycle
260+
continue;
261+
}
190262
}
191263

192264
// Other error (e.g., permission denied, directory doesn't exist)

tests/unit/utils/fileLock.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,9 @@ describe('fileLock utils', () => {
117117
expect(lock1).not.toBeNull();
118118

119119
// Start acquiring second lock (will wait for lock1 to be released)
120-
// Use generous timeout to avoid flakiness on slow systems
120+
// Use generous timeout to avoid flakiness on slow systems and when running in parallel
121121
const startTime = Date.now();
122-
const lock2Promise = acquireLock(filePath, { timeout: 2000, retryInterval: 50 });
122+
const lock2Promise = acquireLock(filePath, { timeout: 5000, retryInterval: 50 });
123123

124124
// Give lock2Promise a moment to start and enter the retry loop
125125
// This ensures it's actively checking before we release lock1
@@ -146,7 +146,7 @@ describe('fileLock utils', () => {
146146

147147
expect(lock2).not.toBeNull();
148148
expect(elapsed).toBeGreaterThanOrEqual(90); // Should have waited
149-
expect(elapsed).toBeLessThan(2000); // Should not have timed out
149+
expect(elapsed).toBeLessThan(5000); // Should not have timed out (increased for parallel test runs)
150150

151151
await lock2!.release();
152152
});

0 commit comments

Comments
 (0)