fix(terminal): resolve pendingDelete race and save contention causing crashes
Two bugs in terminal session persistence caused crashes when terminals were destroyed and recreated with the same ID: 1. pendingDelete blocked legitimate terminal recreation: When a terminal exits and is recreated (worktree switch, shell restart), the 5-second pendingDelete window silently blocked all session saves for the new terminal, leaving it invisible to the session store. Added clearPendingDelete() to remove the guard when createTerminal() is called with a reused ID. 2. Sync save() and async saveAsync() raced on the same temp file: Both methods wrote to terminals.json.tmp without coordination, causing ENOENT errors when one renamed a file the other had already moved. save() now checks writeInProgress and defers to the async writer when a write is in-flight. Co-Authored-By: Claude Opus 4.6 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
5495e6564e
commit
d745aee48d
@@ -195,8 +195,20 @@ export class TerminalSessionStore {
|
||||
* 1. Write to temp file
|
||||
* 2. Rotate current file to backup
|
||||
* 3. Rename temp to target (atomic on most filesystems)
|
||||
*
|
||||
* If an async write is in progress, defers to the async writer to avoid
|
||||
* both operations competing for the same temp file (ENOENT race condition).
|
||||
*/
|
||||
private save(): void {
|
||||
// If an async write is in progress, don't write synchronously — the async
|
||||
// writer shares the same temp file path. Instead, mark a pending write so
|
||||
// saveAsync() will re-save with the latest in-memory data when it finishes.
|
||||
if (this.writeInProgress) {
|
||||
this.writePending = true;
|
||||
debugLog('[TerminalSessionStore] Deferring sync save — async write in progress');
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
const content = JSON.stringify(this.data, null, 2);
|
||||
|
||||
@@ -598,6 +610,27 @@ export class TerminalSessionStore {
|
||||
return sessions.find(s => s.id === sessionId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Clear a session ID from pendingDelete, allowing saves to proceed.
|
||||
*
|
||||
* Called when a terminal is legitimately re-created with the same ID
|
||||
* (e.g., worktree switching, terminal restart after exit). Without this,
|
||||
* the 5-second pendingDelete window blocks session persistence for the
|
||||
* new terminal.
|
||||
*/
|
||||
clearPendingDelete(sessionId: string): void {
|
||||
if (this.pendingDelete.has(sessionId)) {
|
||||
this.pendingDelete.delete(sessionId);
|
||||
// Also clear the cleanup timer since we're explicitly clearing
|
||||
const timer = this.pendingDeleteTimers.get(sessionId);
|
||||
if (timer) {
|
||||
clearTimeout(timer);
|
||||
this.pendingDeleteTimers.delete(sessionId);
|
||||
}
|
||||
debugLog('[TerminalSessionStore] Cleared pendingDelete for re-created terminal:', sessionId);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove a session (from today's sessions)
|
||||
*
|
||||
|
||||
@@ -225,6 +225,18 @@ export function persistAllSessions(terminals: Map<string, TerminalProcess>): voi
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Clear a terminal ID from pendingDelete, allowing session saves to proceed.
|
||||
*
|
||||
* Must be called when re-creating a terminal with a previously-used ID
|
||||
* (e.g., worktree switching, terminal restart after shell exit). Without this,
|
||||
* the pendingDelete guard blocks persistence for the new terminal.
|
||||
*/
|
||||
export function clearPendingDelete(terminalId: string): void {
|
||||
const store = getTerminalSessionStore();
|
||||
store.clearPendingDelete(terminalId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove a session from persistent storage
|
||||
*/
|
||||
|
||||
@@ -54,6 +54,13 @@ export async function createTerminal(
|
||||
return { success: true };
|
||||
}
|
||||
|
||||
// Clear any pendingDelete for this terminal ID. This handles the case where
|
||||
// a terminal is destroyed and immediately re-created with the same ID (e.g.,
|
||||
// worktree switching, terminal restart after shell exit). Without this, the
|
||||
// pendingDelete guard (5-second window) blocks session persistence for the
|
||||
// new terminal, causing it to be invisible to the session store.
|
||||
SessionHandler.clearPendingDelete(id);
|
||||
|
||||
try {
|
||||
// For auth terminals, don't inject existing OAuth token - we want a fresh login
|
||||
const profileEnv = skipOAuthToken ? {} : PtyManager.getActiveProfileEnv();
|
||||
|
||||
Reference in New Issue
Block a user