_connect_via_tcp notification handler reads self._sessions without _sessions_lock
Nobody has claimed this yet.
Assessment
- Difficulty
- 1/5
- Estimated time
- Under an hour
- Newbie friendliness
- 78/100
Research direction
Start in the Python SDK client at _connect_via_tcp around line 4749 and compare its notification handler with _connect_via_stdio around line 4629. Confirm the TCP handler protects its _sessions lookup with _sessions_lock and that session events, including permission requests and tool calls, are still dispatched; no test file is named in the issue.
Written by the indexing model from the issue text.
Description
While reading through the Python SDK client code, I noticed the notification handler inside _connect_via_tcp does a bare self._sessions.get(session_id) without holding self._sessions_lock.
The stdio handler gets this right — it wraps the lookup in with self._sessions_lock: — but the TCP version doesn't. Looks like these two handlers were written separately (or one was copied from the other before the lock was added) and they drifted.
stdio path (_connect_via_stdio, around line 4629):
python
with self._sessions_lock:
session = self._sessions.get(session_id)
TCP path (_connect_via_tcp, around line 4749):
python
session = self._sessions.get(session_id) # no lock
_sessions is mutated from the asyncio event loop thread (create/resume/destroy) and read here from the notification handler that gets scheduled via call_soon_threadsafe from the reader thread. Every other access to _sessions across the file (there are 15+ of them) correctly uses the lock — this is the only one that doesn't.
For what it's worth, the Go SDK's equivalent (handleSessionEvent in client.go) always grabs sessionsMux before touching the sessions map, regardless of transport.
Why it matters
Right now on CPython with the GIL, dict.get() is accidentally atomic so this mostly works by luck. But it's still a logic race — a notification can arrive while a session is mid-registration and get silently dropped.
Since _dispatch_event handles permission requests, tool calls, and MCP OAuth, a dropped event means the session hangs forever waiting for a response that'll never come.
With free-threaded Python (3.13+ nogil builds), this becomes a real data race on the dict internals — potential segfault territory.
Fix
Pretty straightforward one-liner:
diff
def handle_notification(method: str, params: dict):
if method == "session.event":
session_id = params["sessionId"]
event_dict = params["event"]
event = session_event_from_dict(event_dict)
-
session = self._sessions.get(session_id)
-
with self._sessions_lock: -
session = self._sessions.get(session_id) if session: session._dispatch_event(event)
Happy to put up a PR if this looks right.
- Dominant language
- Java
- Stars
- 10.5k
- Forks
- 1.5k
- Avg merge
- 1d 12h
- Merged PRs (30d)
- 133
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from github/copilot-sdk
-
agentic-workflows
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
github/copilot-sdk#2709 · 1 comment ·
-
bug testing
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
github/copilot-sdk#2628 ·
-
agentic-workflows
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
github/copilot-sdk#2627 · 1 comment ·
-
agentic-workflows
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
github/copilot-sdk#2493 ·
-
agentic-workflows
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
github/copilot-sdk#2489 ·
All issues in github/copilot-sdk
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
infinispan/infinispan#18150 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
-
untriaged
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
opensearch-project/k-NN#3597 ·
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 82/100