Skip to content

destroySession can race with concurrent getOrCreateSession during child-exit await #106

@fitz123

Description

@fitz123

Problem

SessionManager.destroySession() closes the child process and deletes the stored session state. During the await this.closeSession(chatId) window (waiting for the child to exit), a concurrent getOrCreateSession call for the same chat can see the still-persisted session in the store, load it, and pass --resume to a new subprocess — effectively resurrecting a session that was supposed to be destroyed.

Current upstream code (bot/src/session-manager.ts)

async destroySession(chatId: string): Promise<void> {
  await this.closeSession(chatId);           // awaits child exit
  this.store.deleteSession(chatId);          // store still has the session until here
  try { cleanupSessionMediaDir(chatId); } catch { /* ignore */ }
}

Race window: between entering closeSession and deleteSession running. closeSession itself persists the final state to the store near its start (`this.store.setSession(chatId, ...)`) before awaiting the child exit. So during the await, the store has a fresh, resumable snapshot and getOrCreateSession will happily resume from it.

Fix (already running in a private workspace fork)

  1. Delete store entry FIRST so concurrent lookups miss.
  2. Call closeSession with { persist: false } so it does not re-persist the final state on the way out.

```ts
async closeSession(chatId: string, { persist = true }: { persist?: boolean } = {}): Promise {
// ...
if (persist) {
this.store.setSession(chatId, this.toSessionState(chatId, session));
}
// ...
}

async destroySession(chatId: string): Promise {
this.store.deleteSession(chatId);
await this.closeSession(chatId, { persist: false });
try { cleanupSessionMediaDir(chatId); } catch { /* ignore */ }
}
```

Evidence: this change has been running in a private workspace fork since 2026-04-02 (commit 2964298) with no observed resurrection regressions, and was the resolution picked during the PR #104 merge conflict.

Acceptance

  • destroySession removes store entry before awaiting child exit
  • closeSession accepts { persist?: boolean } option (default true)
  • Unit test: concurrent destroySession + getOrCreateSession on same chatId — new session must not see resumed state
  • Existing media cleanup on destroy is preserved

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions