Skip to content

Commit 1e1e008

Browse files
marksmeedclaude
andcommitted
fix(pool): claim distinct session slots on Windows
_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>
1 parent a612943 commit 1e1e008

3 files changed

Lines changed: 32 additions & 13 deletions

File tree

telegram_mcp/runtime.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@
5353
except ImportError: # pragma: no cover - Windows fallback
5454
fcntl = None
5555

56+
from telegram_mcp.singleton import try_lock_exclusive
57+
5658
from functools import wraps
5759
import telethon.errors.rpcerrorlist
5860
from sanitize import sanitize_user_content, sanitize_name, sanitize_dict, format_tool_result
@@ -366,11 +368,6 @@ def _parse_session_pool() -> List[str]:
366368

367369
def _acquire_session(pool: List[str]) -> str:
368370
"""Claim the first free session in the pool via an advisory file lock."""
369-
if fcntl is None:
370-
# No advisory locks (e.g. Windows): can't coordinate slots, so use the
371-
# first session. For concurrent clients there, prefer distinct
372-
# TELEGRAM_SESSION_STRING_<LABEL> accounts instead.
373-
return pool[0]
374371
lock_dir = os.path.join(tempfile.gettempdir(), "telegram-mcp-session-locks")
375372
try:
376373
os.makedirs(lock_dir, exist_ok=True)
@@ -380,9 +377,12 @@ def _acquire_session(pool: List[str]) -> str:
380377
digest = hashlib.sha1(session.encode("utf-8")).hexdigest()[:16]
381378
lock_path = os.path.join(lock_dir, f"session-{digest}.lock")
382379
try:
383-
fh = open(lock_path, "w")
384-
fcntl.flock(fh.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB)
380+
# "a+", not "w": on Windows the lock covers the first byte, and
381+
# truncating a file another live client holds is refused.
382+
fh = open(lock_path, "a+")
385383
except OSError:
384+
continue
385+
if not try_lock_exclusive(fh):
386386
# Locked by another live client — try the next session.
387387
try:
388388
fh.close()
@@ -391,6 +391,8 @@ def _acquire_session(pool: List[str]) -> str:
391391
continue
392392
_SESSION_LOCKS.append(fh)
393393
try:
394+
fh.seek(0)
395+
fh.truncate()
394396
fh.write(f"pid={os.getpid()}\n")
395397
fh.flush()
396398
except OSError:

telegram_mcp/singleton.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,14 @@ def _unlock(fh: IO) -> None:
6666
pass
6767

6868

69+
# Public names for the primitives above. The session pool in ``runtime`` needs
70+
# the same "try to take an exclusive lock, don't block" behaviour on both
71+
# platforms, and duplicating the msvcrt/fcntl split there is how one of the two
72+
# copies ends up POSIX-only.
73+
try_lock_exclusive = _try_lock
74+
release_lock = _unlock
75+
76+
6977
DEFAULT_LOCK_DIR = Path(tempfile.gettempdir()) / "telegram-mcp-locks"
7078
DEFAULT_GRACE_SECONDS = 20.0
7179
DEFAULT_POLL_INTERVAL = 0.5

tests/test_session_pool.py

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import pytest
55

66
from telegram_mcp import runner, runtime
7+
from telegram_mcp.singleton import try_lock_exclusive
78

89
# --- _parse_session_pool -----------------------------------------------------
910

@@ -30,13 +31,11 @@ def isolated_lock_dir(tmp_path, monkeypatch):
3031

3132
def _lock_slot(lock_dir, session):
3233
"""Simulate another live client holding the slot for ``session``."""
33-
import fcntl
34-
3534
digest = hashlib.sha1(session.encode("utf-8")).hexdigest()[:16]
3635
path = os.path.join(str(lock_dir), "telegram-mcp-session-locks", f"session-{digest}.lock")
3736
os.makedirs(os.path.dirname(path), exist_ok=True)
38-
fh = open(path, "w")
39-
fcntl.flock(fh.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB)
37+
fh = open(path, "a+")
38+
assert try_lock_exclusive(fh), "test could not take the lock it means to hold"
4039
return fh
4140

4241

@@ -64,9 +63,19 @@ def test_acquire_session_raises_when_pool_exhausted(isolated_lock_dir):
6463
fh.close()
6564

6665

67-
def test_acquire_session_without_fcntl_uses_first(isolated_lock_dir, monkeypatch):
68-
monkeypatch.setattr(runtime, "fcntl", None)
66+
def test_acquire_session_locks_are_visible_to_other_clients(isolated_lock_dir):
67+
"""The claimed slot must actually be locked, on every platform.
68+
69+
Windows previously had no advisory locking here and every client was handed
70+
pool[0], which is precisely the collision the pool exists to prevent.
71+
"""
6972
assert runtime._acquire_session(["AAA", "BBB"]) == "AAA"
73+
digest = hashlib.sha1(b"AAA").hexdigest()[:16]
74+
path = os.path.join(
75+
str(isolated_lock_dir), "telegram-mcp-session-locks", f"session-{digest}.lock"
76+
)
77+
with open(path, "a+") as rival:
78+
assert not try_lock_exclusive(rival)
7079

7180

7281
# --- _discover_accounts prefers the pool for the default account -------------

0 commit comments

Comments
 (0)