anthropics / anthropics/claude-agent-sdk-python

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

Open Beginner friendly
#886 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
8.1k
Forks
1.3k
Avg merge
2d 31m
Merged PRs (30d)
1

Description

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

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in src/claude_agent_sdk/_internal/query.py at Query.close(), especially the cleanup around lines 812-828. Review how exceptions from the transcript-mirror batcher or read-task wait affect later cleanup, then verify that transport.close() still runs when an earlier cleanup step raises. Run the relevant test suite and confirm subprocess and background-task cleanup remains covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.