anthropics / anthropics/claude-agent-sdk-python

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

Aberta
#886 0 comentários 0 reações 0 responsáveis Ver no GitHub
Linguagem predominante
Python
Estrelas
8.1k
Forks
1.3k
Merge médio
2d 31min
PRs com merge (30d)
1

Descrição

## 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`.

Guia de contribuição

Nenhum guia de contribuição indexado para este repositório

Avaliação

Esta issue ainda não foi avaliada.

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.