anthropics / anthropics/claude-agent-sdk-python

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

Đang mở
#886 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
Python
Star
8.1k
Fork
1.3k
Merge trung bình
2 ngày 31 phút
Pull request đã merge (30 ngày)
1

Mô tả

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

Hướng dẫn đóng góp

Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.