Skip to content

fix(python-sdk): return from _iterate_events after end event to unblock wait()#1299

Open
FisherXZ wants to merge 5 commits intoe2b-dev:mainfrom
FisherXZ:fix/1034-kill-blocking
Open

fix(python-sdk): return from _iterate_events after end event to unblock wait()#1299
FisherXZ wants to merge 5 commits intoe2b-dev:mainfrom
FisherXZ:fix/1034-kill-blocking

Conversation

@FisherXZ
Copy link
Copy Markdown

@FisherXZ FisherXZ commented Apr 28, 2026

Issue

Fixes #1034.

AsyncCommandHandle.wait() blocks indefinitely after kill().

Investigation

Traced the hang through the Python async SDK:

handle.kill()
  → Commands.kill(handle.pid)            [commands/__init__.py]
      → send_signal(SIGKILL)             [envd RPC — process is killed]

handle.wait()
  → await self._wait                     [command_handle.py]
      → _handle_events()
          → async for ... in _iterate_events()
              → async for event in self._events   ← BLOCKS HERE

_iterate_events receives the "end" event from envd, sets self._result — then continues the async for loop waiting for the next event. If envd is slow to close the HTTP stream after sending "end", the loop waits indefinitely. wait() blocks with it.

A secondary issue was also found during investigation: even after fixing the blocking, self._events (the acall_server_stream async generator) held its async with http_resp: block open until the handle was garbage-collected. Because HTTP/1.1 is used (not HTTP/2), each unclosed handle tied up one TCP connection. In workloads that accumulate handles, this can exhaust the connection pool (max_connections=2000).

Fix

Commit 1 — stop blocking: Add return after setting self._result in _iterate_events. The generator exits immediately on "end", so _handle_events terminates and wait() unblocks regardless of stream-close timing.

if event.event.HasField("end"):
    self._result = CommandResult(...)
    return  # ← exit generator; stream-close timing no longer matters

Commit 2 — release connection: Add await self._events.aclose() in a finally block in _handle_events. This releases the HTTP connection deterministically in all three paths:

Path Behaviour
Normal "end" received async for ends; finally calls aclose()
Exception during iteration exception caught; finally calls aclose()
disconnect() → task cancelled CancelledError breaks loop; finally calls aclose()

Calling aclose() in the finally of the coroutine that owns the iteration avoids the concurrent-access RuntimeError that's why the # await self._events.aclose() line in disconnect() was commented out.

Two lines changed total. No new state, no new exception types, no behavioural change to kill() or disconnect().

This also applies to PTY handles — AsyncCommandHandle is shared by pty.py, so PTY sessions now terminate promptly and release their connection after the process exits.

Tests

  • test_kill_via_handle — verifies handle.kill() (the instance method) returns True and the process is dead. The existing test_kill_process uses sandbox.commands.kill(pid) — a different code path.
  • test_kill_handle_wait_raises — verifies handle.wait() raises CommandExitException within 5 seconds after handle.kill(). asyncio.wait_for(..., timeout=5.0) is a safety net: if the fix regresses, the test fails with TimeoutError instead of hanging silently.

Out of scope

  • __aiter__ on AsyncCommandHandle — PR fix: add __aiter__ to AsyncCommandHandle for async iteration support #1192 creates a second consumer of self._eventsRuntimeError. Separate issue.
  • AsyncWatchHandle has the identical # await self._events.aclose() pattern. Same fix applies but left for a separate PR to keep scope small.
  • JS SDK — AbortController already handles clean stream cancellation. Unaffected.

🤖 Generated with Claude Code

FisherXZ and others added 4 commits April 21, 2026 16:49
…ck wait()

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…2b-dev#1034

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 28, 2026

🦋 Changeset detected

Latest commit: a6bb6ad

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@e2b/python-sdk Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dbbad45b2c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

exit_code=event.event.end.exit_code,
error=event.event.end.error,
)
return
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Close event stream before returning on end event

Returning immediately here stops consuming self._events, but the stream generator is kept on the handle and is not closed in wait() (and disconnect() does not aclose() it). The async stream implementation (e2b_connect.Client.acall_server_stream) holds the HTTP stream inside an async with, so exiting early without closing/exhausting the generator can keep connections open until the handle is garbage-collected; in workloads that keep many handles, this can accumulate and exhaust the async connection pool.

Useful? React with 👍 / 👎.

…k to release HTTP connection

Without this, returning early from _iterate_events after the "end" event
leaves self._events (acall_server_stream) suspended with its async with
http_resp block still open. The TCP connection is held until the handle
is GC'd. Adding aclose() in finally releases it deterministically in all
paths: normal completion, exception, and cancellation from disconnect().

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Executing AsyncCommandHandle.kill() will cause blocking and cannot kill the process.

1 participant