Skip to content

Commit ebfc896

Browse files
lambdalisueclaude
andcommitted
fix: make session saving asynchronous with proper queue to prevent race conditions
- Remove await from savePickerSession call in defer block to prevent blocking picker close - Implement queue-based session saving to preserve chronological order - Add waitForPendingSaves() for testing to ensure saves complete - Prevent race conditions when multiple pickers are closed quickly - Sessions now save in background while picker closes immediately 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 2c66097 commit ebfc896

File tree

3 files changed

+78
-8
lines changed

3 files changed

+78
-8
lines changed

denops/fall/main/picker.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,15 +212,18 @@ async function startPicker<T extends Detail>(
212212
stack.defer(() => {
213213
zindex -= Picker.ZINDEX_ALLOCATION;
214214
});
215-
stack.defer(async () => {
215+
stack.defer(() => {
216216
const name = pickerParams.name;
217217
if (SESSION_EXCLUDE_SOURCES.includes(name)) {
218218
return;
219219
}
220-
await savePickerSession({
220+
// Save session asynchronously without blocking picker close
221+
savePickerSession({
221222
name,
222223
args,
223224
context: itemPicker.context,
225+
}).catch((error) => {
226+
console.error(`Failed to save picker session: ${error}`);
224227
});
225228
});
226229

denops/fall/session.ts

Lines changed: 63 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,40 @@ const sessions: PickerSessionCompressed<any>[] = [];
1616
*/
1717
const MAX_SESSION_COUNT = 100;
1818

19+
/**
20+
* Queue to ensure session saves happen in order even when compression times vary.
21+
* This prevents race conditions when multiple pickers are closed quickly.
22+
*/
23+
const saveQueue: Array<() => Promise<void>> = [];
24+
let isProcessingQueue = false;
25+
let queueProcessPromise: Promise<void> | null = null;
26+
27+
/**
28+
* Processes the save queue sequentially to maintain chronological order.
29+
*/
30+
function processQueue(): Promise<void> {
31+
if (isProcessingQueue) return queueProcessPromise!;
32+
isProcessingQueue = true;
33+
34+
queueProcessPromise = (async () => {
35+
while (saveQueue.length > 0) {
36+
const saveTask = saveQueue.shift();
37+
if (saveTask) {
38+
try {
39+
await saveTask();
40+
} catch (error) {
41+
console.error(`Failed to save picker session: ${error}`);
42+
}
43+
}
44+
}
45+
46+
isProcessingQueue = false;
47+
queueProcessPromise = null;
48+
})();
49+
50+
return queueProcessPromise;
51+
}
52+
1953
/**
2054
* Represents a picker session with all its state information.
2155
* @template T - The type of item detail in the picker
@@ -101,17 +135,40 @@ export function listPickerSessions(): readonly PickerSessionCompressed<
101135
* Saves a picker session to the in-memory storage.
102136
* The session is compressed before storage to reduce memory usage.
103137
* If the storage exceeds MAX_SESSION_COUNT, the oldest session is removed.
138+
* Sessions are saved in chronological order using a queue to prevent race conditions.
104139
* @template T - The type of item detail in the picker
105140
* @param session - The session to save
106-
* @returns A promise that resolves when the session is saved
141+
* @returns A promise that resolves when the session save is queued (not when it's actually saved)
107142
*/
108-
export async function savePickerSession<T extends Detail>(
143+
export function savePickerSession<T extends Detail>(
109144
session: PickerSession<T>,
110145
): Promise<void> {
111-
const compressed = await compressPickerSession(session);
112-
sessions.push(compressed);
113-
if (sessions.length > MAX_SESSION_COUNT) {
114-
sessions.shift(); // Keep only the last MAX_SESSION_COUNT sessions
146+
// Add the save task to the queue
147+
saveQueue.push(async () => {
148+
const compressed = await compressPickerSession(session);
149+
sessions.push(compressed);
150+
if (sessions.length > MAX_SESSION_COUNT) {
151+
sessions.shift(); // Keep only the last MAX_SESSION_COUNT sessions
152+
}
153+
});
154+
155+
// Process the queue asynchronously
156+
processQueue().catch((error) => {
157+
console.error(`Failed to process save queue: ${error}`);
158+
});
159+
160+
// Return immediately to not block the picker close
161+
return Promise.resolve();
162+
}
163+
164+
/**
165+
* Waits for all pending session saves to complete.
166+
* This is primarily for testing purposes.
167+
* @internal
168+
*/
169+
export async function waitForPendingSaves(): Promise<void> {
170+
if (queueProcessPromise) {
171+
await queueProcessPromise;
115172
}
116173
}
117174

denops/fall/session_test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
loadPickerSession,
77
type PickerSession,
88
savePickerSession,
9+
waitForPendingSaves,
910
} from "./session.ts";
1011
import type { PickerContext } from "./picker.ts";
1112

@@ -46,6 +47,7 @@ Deno.test("session management", async (t) => {
4647
await t.step("savePickerSession stores a session", async () => {
4748
const session = createMockSession("test", 1);
4849
await savePickerSession(session);
50+
await waitForPendingSaves();
4951

5052
const sessions = listPickerSessions();
5153
assertEquals(sessions.length >= 1, true);
@@ -60,6 +62,7 @@ Deno.test("session management", async (t) => {
6062
await savePickerSession(createMockSession("test1", 1));
6163
await savePickerSession(createMockSession("test2", 2));
6264
await savePickerSession(createMockSession("test3", 3));
65+
await waitForPendingSaves();
6366

6467
const sessions = listPickerSessions();
6568
// Most recent session should be first
@@ -73,6 +76,7 @@ Deno.test("session management", async (t) => {
7376
"loadPickerSession retrieves the most recent session by default",
7477
async () => {
7578
await savePickerSession(createMockSession("recent", 99));
79+
await waitForPendingSaves();
7680

7781
const session = await loadPickerSession();
7882
assertExists(session);
@@ -90,6 +94,7 @@ Deno.test("session management", async (t) => {
9094
for (let i = 0; i < 5; i++) {
9195
await savePickerSession(createMockSession(`session${i}`, i));
9296
}
97+
await waitForPendingSaves();
9398

9499
// Index 1 = most recent (session4)
95100
const session0 = await loadPickerSession({ number: 1 });
@@ -115,6 +120,7 @@ Deno.test("session management", async (t) => {
115120
await savePickerSession(createMockSession("file", 3));
116121
await savePickerSession(createMockSession("buffer", 4));
117122
await savePickerSession(createMockSession("file", 5));
123+
await waitForPendingSaves();
118124

119125
// Load most recent "file" session
120126
const fileSession = await loadPickerSession({ name: "file" });
@@ -181,6 +187,7 @@ Deno.test("session management", async (t) => {
181187
};
182188

183189
await savePickerSession(sessionWithCustomContext);
190+
await waitForPendingSaves();
184191
const loadedSession = await loadPickerSession({
185192
name: "compression-test",
186193
});
@@ -213,6 +220,7 @@ Deno.test("session management", async (t) => {
213220
for (let i = 0; i < 105; i++) {
214221
await savePickerSession(createMockSession(`session-limit-${i}`, i));
215222
}
223+
await waitForPendingSaves();
216224

217225
const sessions = listPickerSessions();
218226
// Should only have MAX_SESSION_COUNT sessions
@@ -245,6 +253,7 @@ Deno.test("session management", async (t) => {
245253
};
246254

247255
await savePickerSession(minimalSession);
256+
await waitForPendingSaves();
248257
const loaded = await loadPickerSession({ name: "minimal-context" });
249258

250259
assertExists(loaded);
@@ -279,6 +288,7 @@ Deno.test("session management", async (t) => {
279288
};
280289

281290
await savePickerSession(sessionWithSpecialChars);
291+
await waitForPendingSaves();
282292
const loaded = await loadPickerSession({ name: "special-chars" });
283293

284294
assertExists(loaded);

0 commit comments

Comments
 (0)