Repository navigation
Executing AsyncCommandHandle.kill() will cause blocking and cannot kill the process. #1034
Copy link
Copy link
Open
Labels
Description
Describe the bug
When I also use asyncio.create_task(run_command()) to start a task and get output in real-time, upon catching an exception (such as request disconnection), I want to abort the task, but kill() does not take effect. After pause sandbox and get_info() shows the sandbox status as paused, but the process is still running(It's also possible that I misjudged the situation. The Claude model token costs were still being incurred after the sandbox was paused — this might be because the request had already been sent out when I paused the sandbox.).
Expected behavior
- Be able to successfully kill the process
- When the sandbox is paused, the process should stop executing
- I need an effective method to kill processes. Now when I reconnect to the sandbox, the previous task starts running again.This is causing me to incur a lot of unnecessary token expenses.
Main code
async def run_command():
nonlocal command_handle
try:
logger.info(f"[COMMAND] Starting command with background=True...")
command_handle = await sdx.commands.run(
command,
timeout=600,
on_stdout=on_stdout,
on_stderr=on_stderr,
cwd=work_path,
envs=envs,
user=user,
background=True,
)
logger.info(
f"[COMMAND] Got command handle: {command_handle}, pid: {getattr(command_handle, 'pid', 'N/A')}")
logger.info(f"[COMMAND] Waiting for command to complete...")
result = await command_handle.wait()
logger.info(f"[COMMAND] Command completed with exit_code: {getattr(result, 'exit_code', 'N/A')}")
except asyncio.CancelledError:
logger.warning(f"[COMMAND] run_command task was cancelled")
raise
except Exception as e:
logger.error(f"[COMMAND] Command error: {e}")
finally:
logger.info(f"[COMMAND] run_command finally block, setting done_event")
done_event.set()
try:
run_command_task = asyncio.create_task(run_command())
logger.info(f"[STREAM] Started run_command_task: {run_command_task}")
while not done_event.is_set() or not output_queue.empty():
if await request.is_disconnected():
logger.warning("[STREAM] Client disconnected detected!")
logger.info(f"[STREAM] command_handle exists: {command_handle is not None}")
logger.info(
f"[STREAM] run_command_task done: {run_command_task.done() if run_command_task else 'N/A'}")
if command_handle:
try:
logger.info(
f"[CLEANUP] Attempting to kill command, pid: {getattr(command_handle, 'pid', 'N/A')}")
kill_result = await command_handle.kill()
logger.info(f"[CLEANUP] Kill result: {kill_result}")
except Exception as e:
logger.error(f"[CLEANUP] Error killing command: {type(e).__name__}: {e}")
try:
logger.info(f"[CLEANUP] Attempting to disconnect command handle...")
await command_handle.disconnect()
logger.info(f"[CLEANUP] Disconnect completed successfully")
except Exception as e:
logger.error(f"[CLEANUP] Error disconnecting command: {type(e).__name__}: {e}")
else:
logger.warning(f"[CLEANUP] No command_handle available to kill/disconnect")
if run_command_task and not run_command_task.done():
logger.info(f"[CLEANUP] Cancelling run_command_task...")
run_command_task.cancel()
try:
await run_command_task
logger.info(f"[CLEANUP] run_command_task awaited after cancel")
except asyncio.CancelledError:
logger.info(f"[CLEANUP] run_command_task CancelledError caught (expected)")
else:
logger.info(f"[CLEANUP] run_command_task already done or None")
logger.info(f"[STREAM] Breaking out of main loop due to client disconnect")
break
try:
data = await asyncio.wait_for(output_queue.get(), timeout=0.01)
# Some data processing operations ...
yield processed_data
except asyncio.TimeoutError:
continue
logger.info(
f"[STREAM] Exited main while loop. done_event.is_set(): {done_event.is_set()}, queue empty: {output_queue.empty()}")
except Exception as e:
logger.info(f"[EXCEPTION] command_handle exists: {command_handle is not None}")
if command_handle:
try:
await command_handle.disconnect()
logger.info(f"[EXCEPTION-CLEANUP] Disconnect completed")
except Exception as e:
logger.error(f"[EXCEPTION-CLEANUP] Error disconnecting command: {type(e).__name__}: {e}")
try:
await command_handle.kill()
logger.info(f"[EXCEPTION-CLEANUP] Kill completed")
except Exception as e:
logger.error(f"[EXCEPTION-CLEANUP] Error killing command: {type(e).__name__}: {e}")
# Some data processing operations ...
yield processed_data
finally:
logger.info(f"[FINALLY] Entering finally block. cleanup_done: {cleanup_done}")
logger.info(
f"[FINALLY] command_handle: {command_handle is not None}, run_command_task: {run_command_task is not None}")
if not cleanup_done:
try:
logger.info("[FINALLY] Executing cleanup with shield protection...")
await asyncio.shield(
cleanup_sandbox(sdx, sandbox_info, body, time_start) # pause sandbox and update db data
)
cleanup_done = True
logger.info("[FINALLY] Cleanup completed successfully")
except Exception as e:
logger.error(f"[FINALLY] Critical error during cleanup: {type(e).__name__}: {e}")
else:
logger.info("[FINALLY] Cleanup already done, skipping")
Reactions are currently unavailable
Activity
linear commentedon Nov 25, 2025
ENG-3332
marswong commentedon Nov 27, 2025
fix: add __aiter__ to AsyncCommandHandle for async iteration support
test(python-sdk): add handle.kill() and non-blocking wait() tests for e…
Huaian666 commentedon Jun 6, 2026
jinngimk-lang commentedon Aug 17, 2026
devin-ai-integration commentedon Aug 19, 2026
Triage
Written by Devin
tttboy123 commentedon Sep 3, 2026
I reproduced the process-lifecycle part of this issue against hosted E2B with Python SDK 2.46.4.
I could not reproduce the blocking part on the current SDK. The RPC returned promptly. The observable problem is that
kill()acknowledges signalling the managed PID, but descendants can continue running.The current envd implementation appears consistent with that result:
Handler.SendSignalcallsProcess.Signal/kill(pid, signal), while ordinary non-PTY commands are not started in a command-specific process group. Memory-preserving pause/resume then correctly snapshots and restores any surviving descendants, so I think pause/resume behavior should be treated separately from command-kill semantics.Before implementing, could a maintainer confirm the intended compatibility boundary?
CommandHandle.kill()keep its current PID-only meaning, allowing child processes to survive intentionally, and gain an explicit opt-in tree scope such askill(descendants=True)/killTree()?setsid()can escape it. A command-specific cgroup pluscgroup.killgives a stronger Linux guarantee, at the cost of a larger lifecycle change.My preference is option 1 for backward compatibility: preserve PID-only termination as the default and expose an explicit tree-kill mode. I can implement the selected scope with a regression test covering
leader -> two children, plus equivalent async/sync Python and JS SDK behavior if the public API changes.tttboy123 commentedon Sep 3, 2026
I completed an implementation of the opt-in compatibility design and would like to claim this issue. Please assign #1034 to @tttboy123 if this scope matches the maintainers' intent.
PRs:
Confirmed problem
On hosted E2B with Python SDK 2.46.4,
CommandHandle.kill()returnedtruein 0.322 seconds and the managed shell disappeared, but its two child processes remained alive. They were still alive afterpause(keep_memory=True)andresume().I did not reproduce the original indefinite blocking report on the current SDK. The confirmed failure is narrower: a successful leader-only signal can look like complete command termination while descendants continue working. Memory-preserving pause/resume then correctly restores those surviving processes; changing pause/resume is not part of this fix.
Implemented solution
The existing behavior stays unchanged by default, because keeping subprocesses after the parent exits can be intentional. Callers can explicitly request tree termination:
The envd PR starts managed commands in command-owned process groups and, only for the opt-in request, sends
SIGKILLto that group. It validates that the managed PID owns the group before using a negative PGID, avoiding any possibility of signalling envd's own group. PTY commands already use their own session/process group. The default path continues to signal only the selected PID. It also keeps output connected when signal validation or delivery fails, and cancels the output pumps only after a terminal signal is successfully delivered.The SDK PR exposes
scope: "process" | "group"consistently across sync/async Python, JS, command handles, direct command APIs, and PTY APIs. It rejectsgroupon envd versions older than 0.7.1; otherwise protobuf's unknown-field behavior could silently degrade the request into leader-only termination. It also validates runtime values before the RPC, so a typo such asgroupscannot silently fall back to the default process scope.Verification
go test -race ./...: passThe full SDK test projects include live cloud fixtures. I did not count those as passing evidence because the new envd behavior is not deployed yet and no cloud key was injected into the test commands.
Proposed follow-up boundary
This process-group solution covers the reproduced
shell -> childrencase while preserving compatibility. A descendant can deliberately escape withsetsid(). If the product contract should guarantee that every descendant is gone regardless of session changes, I propose a separate follow-up design around a per-command cgroup andcgroup.kill, including lifecycle cleanup and live-upgrade behavior. That is materially broader and should not be hidden inside the compatibility fix.I am happy to adjust naming or scope based on maintainer feedback. If this API boundary is accepted, the SDK PR can be reviewed after the envd dependency lands and is assigned version 0.7.1.
tttboy123 commentedon Sep 3, 2026
Follow-up after the SDK design review: I changed the public option from a boolean to an explicit scope and pushed the latest hardening in #1801 (
5ae8eea47).The default is
process.groupmaps to the envddescendantswire field. This names the actual guarantee accurately—the process group, not every possible descendant—and leaves room for a future strongertree/cgroup scope without turning a boolean into a breaking API.The revision also:
Fresh validation: Python command/PTY/handle tests 23/23, Python offline top-level unit tests 301/301, JS command/PTY/handle tests 19/19, and both SDKs' build/lint/type checks pass.
make codegenalso passes and leaves no additional generated drift.The compatibility and future-work boundary is unchanged: existing process-only termination remains the default; a future cgroup-backed
treescope can provide a guarantee across descendants that callsetsid().tttboy123 commentedon Oct 6, 2026
Backend refresh is now pushed: e2b-dev/runtime#3617 at
9c53269f0is conflict-free against the checked upstream and CLA is passing. The October 5 validation and its limitations are recorded at e2b-dev/runtime#3617 (comment).The compatibility boundary is unchanged: default explicit kill targets only the leader; callers may opt into the owned process group. This does not change memory-preserving pause/resume or destroy the sandbox, and it does not yet implement automatic-timeout group cleanup (separately proposed in #1031).
One correction to my original version note:
0.7.1was a proposed floor, not a released guarantee for this feature. The refreshed backend proposes0.9.1; the final SDK feature floor must match the actual backend release. SDK #1801 remains Draft and untouched until the backend merges, per the maintainer's request. It will need version-gate tests and revalidation before readiness.