refactor: extract shared file lock utilities
Extract the duplicated file lock mechanism from auth-storage.ts and session-manager.ts into a shared lock-utils.ts module. - acquireLockSyncWithRetry(): throwing variant (used by auth-storage) - tryAcquireLockSync(): non-throwing variant (used by session-manager) - acquireLockAsync(): async lock with retries and staleness detection Removes ~55 lines of duplicated retry-loop logic. The shared module also provides a foundation for deduplicating identical patterns in settings-manager.ts and models-json-writer.ts.
This commit is contained in:
frizynn
parent
eaf0538150
commit
23d0ea656d
3 changed files with 120 additions and 68 deletions
|
|
@ -18,9 +18,9 @@ import {
|
|||
import { getOAuthApiKey, getOAuthProvider, getOAuthProviders } from "@gsd/pi-ai/oauth";
|
||||
import { chmodSync, existsSync, mkdirSync, readFileSync, writeFileSync } from "fs";
|
||||
import { dirname, join } from "path";
|
||||
import lockfile from "proper-lockfile";
|
||||
import { getAgentDir } from "../config.js";
|
||||
import { AUTH_LOCK_STALE_MS } from "./constants.js";
|
||||
import { acquireLockAsync, acquireLockSyncWithRetry } from "./lock-utils.js";
|
||||
import { resolveConfigValue } from "./resolve-config-value.js";
|
||||
|
||||
export type ApiKeyCredential = {
|
||||
|
|
@ -67,40 +67,13 @@ export class FileAuthStorageBackend implements AuthStorageBackend {
|
|||
}
|
||||
}
|
||||
|
||||
private acquireLockSyncWithRetry(path: string): () => void {
|
||||
const maxAttempts = 10;
|
||||
const delayMs = 20;
|
||||
let lastError: unknown;
|
||||
|
||||
for (let attempt = 1; attempt <= maxAttempts; attempt++) {
|
||||
try {
|
||||
return lockfile.lockSync(path, { realpath: false });
|
||||
} catch (error) {
|
||||
const code =
|
||||
typeof error === "object" && error !== null && "code" in error
|
||||
? String((error as { code?: unknown }).code)
|
||||
: undefined;
|
||||
if (code !== "ELOCKED" || attempt === maxAttempts) {
|
||||
throw error;
|
||||
}
|
||||
lastError = error;
|
||||
const start = Date.now();
|
||||
while (Date.now() - start < delayMs) {
|
||||
// Sleep synchronously to avoid changing callers to async.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
throw (lastError as Error) ?? new Error("Failed to acquire auth storage lock");
|
||||
}
|
||||
|
||||
withLock<T>(fn: (current: string | undefined) => LockResult<T>): T {
|
||||
this.ensureParentDir();
|
||||
this.ensureFileExists();
|
||||
|
||||
let release: (() => void) | undefined;
|
||||
try {
|
||||
release = this.acquireLockSyncWithRetry(this.authPath);
|
||||
release = acquireLockSyncWithRetry(this.authPath);
|
||||
const current = existsSync(this.authPath) ? readFileSync(this.authPath, "utf-8") : undefined;
|
||||
const { result, next } = fn(current);
|
||||
if (next !== undefined) {
|
||||
|
|
@ -129,15 +102,8 @@ export class FileAuthStorageBackend implements AuthStorageBackend {
|
|||
};
|
||||
|
||||
try {
|
||||
release = await lockfile.lock(this.authPath, {
|
||||
retries: {
|
||||
retries: 10,
|
||||
factor: 2,
|
||||
minTimeout: 100,
|
||||
maxTimeout: 10000,
|
||||
randomize: true,
|
||||
},
|
||||
stale: AUTH_LOCK_STALE_MS,
|
||||
release = await acquireLockAsync(this.authPath, {
|
||||
staleMs: AUTH_LOCK_STALE_MS,
|
||||
onCompromised: (err) => {
|
||||
lockCompromised = true;
|
||||
lockCompromisedError = err;
|
||||
|
|
|
|||
113
packages/pi-coding-agent/src/core/lock-utils.ts
Normal file
113
packages/pi-coding-agent/src/core/lock-utils.ts
Normal file
|
|
@ -0,0 +1,113 @@
|
|||
/**
|
||||
* Shared file-locking utilities built on `proper-lockfile`.
|
||||
*
|
||||
* Centralises the synchronous retry-loop and async lock/release patterns
|
||||
* that were previously duplicated across auth-storage, session-manager,
|
||||
* settings-manager, and models-json-writer.
|
||||
*/
|
||||
|
||||
import lockfile from "proper-lockfile";
|
||||
|
||||
const DEFAULT_MAX_ATTEMPTS = 10;
|
||||
const DEFAULT_DELAY_MS = 20;
|
||||
|
||||
/**
|
||||
* Acquire a synchronous file lock with retry.
|
||||
*
|
||||
* Retries up to `maxAttempts` times when the lock is held by another process
|
||||
* (ELOCKED), using a busy-wait between attempts.
|
||||
*
|
||||
* @returns A release function to unlock.
|
||||
* @throws On non-ELOCKED errors or when all attempts are exhausted.
|
||||
*/
|
||||
export function acquireLockSyncWithRetry(
|
||||
lockPath: string,
|
||||
maxAttempts: number = DEFAULT_MAX_ATTEMPTS,
|
||||
delayMs: number = DEFAULT_DELAY_MS,
|
||||
): () => void {
|
||||
let lastError: unknown;
|
||||
|
||||
for (let attempt = 1; attempt <= maxAttempts; attempt++) {
|
||||
try {
|
||||
return lockfile.lockSync(lockPath, { realpath: false });
|
||||
} catch (error) {
|
||||
const code =
|
||||
typeof error === "object" && error !== null && "code" in error
|
||||
? String((error as { code?: unknown }).code)
|
||||
: undefined;
|
||||
if (code !== "ELOCKED" || attempt === maxAttempts) {
|
||||
throw error;
|
||||
}
|
||||
lastError = error;
|
||||
const start = Date.now();
|
||||
while (Date.now() - start < delayMs) {
|
||||
// Busy-wait to avoid changing callers to async.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
throw (lastError as Error) ?? new Error("Failed to acquire file lock");
|
||||
}
|
||||
|
||||
/**
|
||||
* Non-throwing variant of {@link acquireLockSyncWithRetry}.
|
||||
*
|
||||
* Returns `undefined` instead of throwing when the lock cannot be acquired,
|
||||
* allowing callers to proceed without the lock rather than losing data.
|
||||
*/
|
||||
export function tryAcquireLockSync(
|
||||
lockPath: string,
|
||||
maxAttempts: number = DEFAULT_MAX_ATTEMPTS,
|
||||
delayMs: number = DEFAULT_DELAY_MS,
|
||||
): (() => void) | undefined {
|
||||
for (let attempt = 1; attempt <= maxAttempts; attempt++) {
|
||||
try {
|
||||
return lockfile.lockSync(lockPath, { realpath: false });
|
||||
} catch (error) {
|
||||
const code =
|
||||
typeof error === "object" && error !== null && "code" in error
|
||||
? String((error as { code?: unknown }).code)
|
||||
: undefined;
|
||||
if (code !== "ELOCKED" || attempt === maxAttempts) {
|
||||
// Non-fatal: proceed without lock rather than losing data
|
||||
return undefined;
|
||||
}
|
||||
const start = Date.now();
|
||||
while (Date.now() - start < delayMs) {
|
||||
// Busy-wait to avoid changing callers to async.
|
||||
}
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
export interface AsyncLockOptions {
|
||||
/** Maximum staleness in ms before the lock is considered stale. */
|
||||
staleMs?: number;
|
||||
/** Called if the lock is compromised while held. */
|
||||
onCompromised?: (err: Error) => void;
|
||||
}
|
||||
|
||||
/**
|
||||
* Acquire an async file lock with retries and optional staleness detection.
|
||||
*
|
||||
* Uses `proper-lockfile`'s async API with exponential-backoff retries.
|
||||
*
|
||||
* @returns A release function (async) to unlock.
|
||||
*/
|
||||
export async function acquireLockAsync(
|
||||
lockPath: string,
|
||||
options?: AsyncLockOptions,
|
||||
): Promise<() => Promise<void>> {
|
||||
return lockfile.lock(lockPath, {
|
||||
retries: {
|
||||
retries: 10,
|
||||
factor: 2,
|
||||
minTimeout: 100,
|
||||
maxTimeout: 10000,
|
||||
randomize: true,
|
||||
},
|
||||
stale: options?.staleMs,
|
||||
onCompromised: options?.onCompromised,
|
||||
});
|
||||
}
|
||||
|
|
@ -16,8 +16,8 @@ import {
|
|||
import { atomicWriteFileSync } from "./fs-utils.js";
|
||||
import { readdir, readFile, stat } from "fs/promises";
|
||||
import { join, resolve } from "path";
|
||||
import lockfile from "proper-lockfile";
|
||||
import { getAgentDir as getDefaultAgentDir, getBlobsDir, getSessionsDir } from "../config.js";
|
||||
import { tryAcquireLockSync } from "./lock-utils.js";
|
||||
import {
|
||||
type BashExecutionMessage,
|
||||
type CustomMessage,
|
||||
|
|
@ -955,39 +955,12 @@ export class SessionManager {
|
|||
}
|
||||
}
|
||||
|
||||
private acquireSessionLock(path: string): (() => void) | undefined {
|
||||
const maxAttempts = 10;
|
||||
const delayMs = 20;
|
||||
let lastError: unknown;
|
||||
|
||||
for (let attempt = 1; attempt <= maxAttempts; attempt++) {
|
||||
try {
|
||||
return lockfile.lockSync(path, { realpath: false });
|
||||
} catch (error) {
|
||||
const code =
|
||||
typeof error === "object" && error !== null && "code" in error
|
||||
? String((error as { code?: unknown }).code)
|
||||
: undefined;
|
||||
if (code !== "ELOCKED" || attempt === maxAttempts) {
|
||||
// Non-fatal: proceed without lock rather than losing data
|
||||
return undefined;
|
||||
}
|
||||
lastError = error;
|
||||
const start = Date.now();
|
||||
while (Date.now() - start < delayMs) {
|
||||
// Busy-wait to avoid async
|
||||
}
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
private _rewriteFile(): void {
|
||||
if (!this.persist || !this.sessionFile) return;
|
||||
const content = `${this.fileEntries.map((e) => JSON.stringify(e)).join("\n")}\n`;
|
||||
let release: (() => void) | undefined;
|
||||
try {
|
||||
release = this.acquireSessionLock(this.sessionFile);
|
||||
release = tryAcquireLockSync(this.sessionFile);
|
||||
atomicWriteFileSync(this.sessionFile, content);
|
||||
} finally {
|
||||
release?.();
|
||||
|
|
@ -1026,7 +999,7 @@ export class SessionManager {
|
|||
|
||||
let release: (() => void) | undefined;
|
||||
try {
|
||||
release = this.acquireSessionLock(this.sessionFile);
|
||||
release = tryAcquireLockSync(this.sessionFile);
|
||||
if (!this.flushed) {
|
||||
for (const e of this.fileEntries) {
|
||||
const prepared = prepareForPersistence(e, this.blobStore) as FileEntry;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue