anthropics / anthropics/claude-agent-sdk-python

Query.close(): transport.close() not guarded by try/finally — can be skipped if earlier cleanup raises

Abierto
#886 0 comentarios 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
Python
Estrellas
8.1k
Forks
1.3k
Merge medio
2 d 31 min
PR fusionados (30 d)
1

Descripción

## Location

`src/claude_agent_sdk/_internal/query.py:828` (on `main` @ `e21b457`)

## Problem

`await self.transport.close()` is the **last** statement in `Query.close()`, not inside a `try/finally`. If any of the preceding cleanup steps (lines ~812-827: transcript-mirror-batcher close, child-task cancel, read-task `wait()`) raise, `transport.close()` is never reached — the stderr reader task and the CLI subprocess are not cleaned up.

```python
async def close(self) -> None:
...
if self._transcript_mirror_batcher is not None:
await self._transcript_mirror_batcher.close()
for task in list(self._child_tasks):
task.cancel()
if self._read_task is not None and not self._read_task.done():
self._read_task.cancel()
await self._read_task.wait()
self._read_task = None
...
self._message_send.close()
await self.transport.close() # <-- skipped if anything above raises
```

## Impact

Potential subprocess / background-task leak on error paths during shutdown. Low likelihood in practice (the preceding calls are mostly non-raising), but `_transcript_mirror_batcher.close()` and `_read_task.wait()` are `await`s that could in principle propagate.

## Suggested fix

Wrap the earlier cleanup steps in `try: ... finally: await self.transport.close()`, or use nested `try/finally` (or an `AsyncExitStack`) so each cleanup step runs regardless of earlier failures. Example:

```python
try:
if self._transcript_mirror_batcher is not None:
await self._transcript_mirror_batcher.close()
for task in list(self._child_tasks):
task.cancel()
if self._read_task is not None and not self._read_task.done():
self._read_task.cancel()
await self._read_task.wait()
self._read_task = None
self._message_send.close()
finally:
await self.transport.close()
```

## Note

Discovered during review of PR #885, which does **not** introduce this — it's pre-existing on `main`.

Guía de contribución

No hay ninguna guía de contribución indexada para este repositorio

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.