Skip to content

fix(pool): claim distinct session slots on Windows - #198

Merged
chigwell merged 1 commit into
chigwell:mainfrom
marksmeed:feature/windows-session-pool
Aug 21, 2026
Merged

fix(pool): claim distinct session slots on Windows#198
chigwell merged 1 commit into
chigwell:mainfrom
marksmeed:feature/windows-session-pool

Conversation

@marksmeed

Copy link
Copy Markdown
Contributor

Follow-up to #172, which fixed the same POSIX-only pattern in the session lock. The session pool still has it.

The bug

_acquire_session guards its advisory locking behind if fcntl is None and returns pool[0]:

if fcntl is None:
    # No advisory locks (e.g. Windows): can't coordinate slots, so use the
    # first session.
    return pool[0]

fcntl is POSIX-only, so on Windows every concurrent client is handed the same session string. That is exactly the collision the pool exists to prevent — Telegram sees one auth key on two connections and revokes it permanently, for both clients. Configuring a pool on Windows today therefore looks like a fix while reproducing the original fault.

This is not theoretical: some MCP hosts run a separate copy of the server per session, so two live clients against one account is the normal case, not an edge case.

The fix

singleton.py already solves cross-platform advisory locking (msvcrt on Windows, fcntl on POSIX). This exposes those primitives as try_lock_exclusive / release_lock and has the pool use them, rather than keeping a second, POSIX-only copy of the same idea.

Two details the Windows path needs:

  • the lock file opens "a+" rather than "w" — the lock covers the first byte, and truncating a file another live client holds is refused
  • the pid marker is rewritten in place instead of appended to on every start

Tests

tests/test_session_pool.py simulated a rival client via a direct import fcntl, so it could not run on Windows at all (ModuleNotFoundError). It now uses the same primitive as the code under test.

test_acquire_session_without_fcntl_uses_first asserted the buggy behaviour, so it is replaced by test_acquire_session_locks_are_visible_to_other_clients, which asserts the claimed slot is genuinely locked against another client.

Verified on Windows 11 / Python 3.13:

  • tests/test_session_pool.py: 9 passed (previously 2 errored on the fcntl import)
  • full suite: 275 passed, up from 273; the remaining failures are pre-existing POSIX file-mode assertions unrelated to this change
  • two processes against a 2-session pool claim distinct slots
  • a third process is refused with "All 2 pooled Telegram session(s) are already claimed", rather than being handed a duplicate

🤖 Generated with Claude Code

_acquire_session guarded its advisory locking behind `if fcntl is None`
and returned pool[0] on Windows. Every concurrent client there was handed
the same session string, which is precisely the collision the pool exists
to prevent: Telegram sees one auth key on two connections and revokes it
permanently for both clients.

singleton.py already solves this, locking through msvcrt on Windows and
fcntl on POSIX, so expose those primitives as try_lock_exclusive/
release_lock and have the pool use them rather than keeping a second,
POSIX-only copy of the same idea.

Two details the Windows path needs: the lock file is opened "a+" instead
of "w", because the lock covers the first byte and truncating a file
another live client holds is refused; and the pid marker is rewritten in
place rather than appended to on every start.

tests/test_session_pool.py simulated a rival client with a direct fcntl
import, so it could not run on Windows at all. It now uses the same
primitive as the code under test. The case asserting that a missing fcntl
yields pool[0] encoded the bug, so it is replaced by one asserting the
claimed slot is genuinely locked against another client.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@chigwell
chigwell merged commit 8eb465c into chigwell:main Aug 21, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants