_connect_via_tcp notification handler reads self._sessions without _sessions_lock
Nadie ha tomado este issue todavía.
Evaluación
- Dificultad
- 1/5
- Tiempo estimado
- Menos de una hora
- Aptitud para principiantes
- 78/100
Línea de trabajo
Comienza en el cliente del SDK de Python, en _connect_via_tcp alrededor de la línea 4749, y compara su controlador de notificaciones con _connect_via_stdio alrededor de la línea 4629. Confirma que el controlador TCP protege su búsqueda en _sessions con _sessions_lock y que los eventos de sesión, incluidas las solicitudes de permisos y las llamadas a herramientas, siguen siendo despachados; en la issue no se nombra ningún archivo de prueba.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
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.
- Lenguaje dominante
- Java
- Estrellas
- 10.5k
- Forks
- 1.5k
- Merge medio
- 1 d 12 h
- PR fusionados (30 d)
- 133
Guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de github/copilot-sdk
-
agentic-workflows
Dificultad 2/5 1-3 horas Aptitud para principiantes 68/100
github/copilot-sdk#2709 · 1 comentario ·
-
bug testing
Dificultad 2/5 1-3 horas Aptitud para principiantes 75/100
github/copilot-sdk#2628 ·
-
agentic-workflows
Dificultad 2/5 1-3 horas Aptitud para principiantes 68/100
github/copilot-sdk#2627 · 1 comentario ·
-
agentic-workflows
Dificultad 2/5 1-3 horas Aptitud para principiantes 74/100
github/copilot-sdk#2493 ·
-
agentic-workflows
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
github/copilot-sdk#2489 ·
Todos los issues de github/copilot-sdk
Issues similares
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
infinispan/infinispan#18150 ·
-
area/frontend
Dificultad 2/5 1-3 horas Aptitud para principiantes 65/100
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 84/100
-
untriaged
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
opensearch-project/k-NN#3597 ·
-
bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 88/100