Skip to content

Commit b852a72

Browse files
cb1kenobiclaude
authored andcommitted
Widen atomicWriteFile's Windows rename-retry budget and sleep without spinning
The ~910ms backoff budget (8 retries, 200ms cap) was exhausted twice in a row by the same test on a Windows CI runner (harper#2036) - an AV real-time scan can hold harper-config.yaml for over a second. Widen to 12 retries with a 500ms cap (~3.6s worst case), and replace the performance.now() busy-spin with a timeout-only Atomics.wait so the wait is CPU-idle, which is what makes the longer budget affordable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent b58db1b commit b852a72

2 files changed

Lines changed: 15 additions & 12 deletions

File tree

config/configUtils.ts

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import YAML from 'yaml';
77
import path from 'path';
88
import { threadId } from 'node:worker_threads';
99
import { randomBytes } from 'node:crypto';
10-
import { performance } from 'node:perf_hooks';
1110
import isNumber from 'is-number';
1211
import propertiesReaderModule from 'properties-reader';
1312
import _ from 'lodash';
@@ -87,10 +86,15 @@ export function getConfigPath(param: string) {
8786
// Every worker thread runs its own RootConfigWatcher (chokidar), so a write on one thread
8887
// routinely races a hot-reload read on another; Windows Defender / AV real-time scanning can
8988
// hold a similar transient handle. Retry with exponential backoff to ride out the race -
90-
// callers are synchronous, so the wait is a synchronous busy-loop rather than a real sleep.
91-
const RENAME_RETRY_MAX_ATTEMPTS = 8;
89+
// callers are synchronous, so the wait is a synchronous sleep rather than an async one.
90+
// The budget must outlast a single AV real-time scan pass (seconds, not hundreds of ms):
91+
// the previous ~910ms budget was exhausted twice in a row by the same test on a CI runner
92+
// (harper#2036), so the worst case is now ~3.6s.
93+
const RENAME_RETRY_MAX_ATTEMPTS = 12;
9294
const RENAME_RETRY_INITIAL_DELAY_MS = 10;
93-
const RENAME_RETRY_MAX_DELAY_MS = 200;
95+
const RENAME_RETRY_MAX_DELAY_MS = 500;
96+
// Never notified; exists only so Atomics.wait can time out (a synchronous, CPU-idle sleep).
97+
const renameRetrySleepBuffer = new Int32Array(new SharedArrayBuffer(4));
9498

9599
function atomicWriteFile(
96100
filePath,
@@ -112,11 +116,10 @@ function atomicWriteFile(
112116
} catch (err) {
113117
if (retries > 0 && (err.code === 'EPERM' || err.code === 'EACCES')) {
114118
retries--;
115-
// Sleep synchronously (all call sites are sync) to allow the reader to close the
116-
// file. Uses performance.now() rather than Date.now(): the latter tracks wall-clock
117-
// time and can jump backward (NTP sync), which would turn this into an unbounded spin.
118-
const start = performance.now();
119-
while (performance.now() - start < delayMs) {}
119+
// Sleep synchronously (all call sites are sync) to allow the holder to close the
120+
// file. Atomics.wait yields the thread to the OS instead of spinning the CPU,
121+
// which is what makes a multi-second worst-case budget affordable.
122+
if (delayMs > 0) Atomics.wait(renameRetrySleepBuffer, 0, 0, delayMs);
120123
delayMs = Math.min(delayMs * 2, maxDelayMs);
121124
continue;
122125
}

unitTests/config/configUtils.test.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -246,12 +246,12 @@ describe('Test configUtils module', () => {
246246
try {
247247
// Use the default retry count (unspecified maxRetries) so this exercises the real
248248
// production budget, but override the delay to ~0 so the backoff doesn't burn real
249-
// wall-clock time (default backoff would take ~900ms for a persistent failure).
249+
// wall-clock time (default backoff would take ~3.6s for a persistent failure).
250250
expect(() => atomicWriteFile(ATOMIC_TEST_PATH, 'content', { initialDelayMs: 0, maxDelayMs: 0 })).to.throw(
251251
epermError
252252
);
253-
// 1 initial attempt + 8 retries (the production default maxRetries)
254-
expect(renameStub.callCount).to.equal(9);
253+
// 1 initial attempt + 12 retries (the production default maxRetries)
254+
expect(renameStub.callCount).to.equal(13);
255255
const stragglers = fs
256256
.readdirSync(ATOMIC_TEST_DIR)
257257
.filter((e) => e.startsWith('atomic-write-test.yaml.') && e.endsWith('.tmp'));

0 commit comments

Comments
 (0)