Skip to content

Commit 79e1034

Browse files
committed
Queue same-process credential writes so each gets its own lock window
Same-process writers all polled the on-disk lock against a deadline that started at call time, so one lock held past the timeout failed the whole burst instead of just the first waiter. Writes now chain per auth file, and temp paths get a per-call counter so two saves in one process can never share one.
1 parent 36d5369 commit 79e1034

2 files changed

Lines changed: 96 additions & 4 deletions

File tree

src/auth/store.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,69 @@ describe("createAuthStore", () => {
114114
}
115115
});
116116

117+
test("gives queued same-process writes their own lock window", async () => {
118+
const home = await mkdtemp(join(tmpdir(), "oauth-store-queue-"));
119+
try {
120+
const store = createAuthStore<TestTokens>({
121+
filename: "test-auth.json",
122+
settingsDirName: TEST_SETTINGS_DIR,
123+
isTokens: isTestTokens,
124+
});
125+
await store.saveProfile(
126+
{
127+
name: "work",
128+
tokens: { access: "a", refresh: "r", expiresAt: 1 },
129+
createdAt: 1,
130+
},
131+
home,
132+
);
133+
134+
// A foreign process holds the credential lock past the first waiter's
135+
// deadline, then releases; the write queued behind it must still land.
136+
const lockPath = `${store.authPath(home)}.lock`;
137+
await writeFile(lockPath, "foreign", { mode: 0o600 });
138+
139+
const first = store
140+
.updateTokens(
141+
"work",
142+
{ access: "first", refresh: "r1", expiresAt: 2 },
143+
home,
144+
)
145+
.then(
146+
() => "resolved" as const,
147+
(error: unknown) => error,
148+
);
149+
const second = store
150+
.updateTokens(
151+
"work",
152+
{ access: "second", refresh: "r2", expiresAt: 3 },
153+
home,
154+
)
155+
.then(
156+
() => "resolved" as const,
157+
(error: unknown) => error,
158+
);
159+
160+
await Bun.sleep(1_400);
161+
await rm(lockPath, { force: true });
162+
163+
const firstResult = await first;
164+
expect(firstResult).toBeInstanceOf(Error);
165+
if (firstResult instanceof Error) {
166+
expect(firstResult.message).toContain(
167+
"Timed out waiting for OAuth credential lock",
168+
);
169+
}
170+
expect(await second).toBe("resolved");
171+
172+
expect((await store.loadProfile("work", home))?.tokens.access).toBe(
173+
"second",
174+
);
175+
} finally {
176+
await rm(home, { recursive: true, force: true });
177+
}
178+
});
179+
117180
test("round-trips profiles under an injected home and survives corrupt files", async () => {
118181
const home = await mkdtemp(join(tmpdir(), "oauth-store-"));
119182
try {

src/auth/store.ts

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,15 @@ interface AuthFile<TTokens extends BaseTokens> {
4848
const LOCK_RETRY_MS = 25;
4949
const LOCK_TIMEOUT_MS = 1_000;
5050

51+
// pid alone is not unique per call — concurrent saves in one process must not
52+
// share a temp path or the second rename hits ENOENT after the first moves it.
53+
let tmpWriteCounter = 0;
54+
55+
// Same-process ops on one auth file queue here so a caller's lock deadline
56+
// starts when it actually runs, not when it was invoked — otherwise one lock
57+
// held past LOCK_TIMEOUT_MS fails the whole burst, not just the first waiter.
58+
const updateChains = new Map<string, Promise<unknown>>();
59+
5160
const AuthFileShape = type({
5261
profiles: "Record<string, unknown>",
5362
});
@@ -115,7 +124,7 @@ export function createAuthStore<TTokens extends BaseTokens>(
115124
): Promise<void> {
116125
const path = authPath(home);
117126
await mkdir(dirname(path), { recursive: true, mode: 0o700 });
118-
const tmp = `${path}.${String(process.pid)}.tmp`;
127+
const tmp = `${path}.${process.pid}.${(tmpWriteCounter += 1)}.tmp`;
119128
await writeFile(tmp, JSON.stringify(file, null, 2), { mode: 0o600 });
120129
await rename(tmp, path);
121130
}
@@ -158,6 +167,26 @@ export function createAuthStore<TTokens extends BaseTokens>(
158167
}
159168
}
160169

170+
function enqueueAuthFileOp<TResult>(
171+
home: string,
172+
op: () => Promise<TResult>,
173+
): Promise<TResult> {
174+
const path = authPath(home);
175+
const previous = updateChains.get(path) ?? Promise.resolve();
176+
const run = previous.then(
177+
() => withAuthFileLock(home, op),
178+
() => withAuthFileLock(home, op),
179+
);
180+
updateChains.set(
181+
path,
182+
run.then(
183+
() => undefined,
184+
() => undefined,
185+
),
186+
);
187+
return run;
188+
}
189+
161190
return {
162191
authPath,
163192
async listProfiles(
@@ -179,7 +208,7 @@ export function createAuthStore<TTokens extends BaseTokens>(
179208
profile: AuthProfile<TTokens>,
180209
home: string = homedir(),
181210
): Promise<void> {
182-
await withAuthFileLock(home, async () => {
211+
await enqueueAuthFileOp(home, async () => {
183212
const file = await readAuthFile(home);
184213
file.profiles[profile.name] = profile;
185214
await writeAuthFile(file, home);
@@ -192,7 +221,7 @@ export function createAuthStore<TTokens extends BaseTokens>(
192221
tokens: TTokens,
193222
home: string = homedir(),
194223
): Promise<void> {
195-
await withAuthFileLock(home, async () => {
224+
await enqueueAuthFileOp(home, async () => {
196225
const file = await readAuthFile(home);
197226
const existing = file.profiles[name];
198227
if (existing === undefined) return;
@@ -204,7 +233,7 @@ export function createAuthStore<TTokens extends BaseTokens>(
204233
name: string | undefined,
205234
home: string = homedir(),
206235
): Promise<string[]> {
207-
return withAuthFileLock(home, async () => {
236+
return enqueueAuthFileOp(home, async () => {
208237
const file = await readAuthFile(home);
209238
if (name === undefined) {
210239
const removed = Object.keys(file.profiles);

0 commit comments

Comments
 (0)