Repository navigation
Keep persistent sandbox sessions responsive over HTTP #528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
abhishekraok
wants to merge
15
commits into
main
Choose a base branch
from
codex/oct31-483-sessions
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+812
−7
Open
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
2acdcaa
Keep sandbox HTTP command execution responsive
abhishekraok f45c977
Settle output readers before command cancellation closes their files
abhishekraok cee8b8c
Wrap the background command regression fixture
abhishekraok 820a21a
Harden remote command startup and deadlines across Python runtimes
abhishekraok 493f4e2
Run persistent shell operations without blocking sandbox HTTP
abhishekraok 22077ab
Merge branch 'codex/oct31-483' into codex/oct31-483-sessions
abhishekraok 8da9322
Bound session interruption and serialize lifecycle mutations
abhishekraok 7905755
Share sandbox container-routing predicates
abhishekraok 361fc56
Merge branch 'codex/oct31-483' into codex/oct31-483-sessions
abhishekraok a4be0e1
Finish session interrupt cleanup after reader deadlines
abhishekraok 9db5e95
Keep cancellation and session state when interrupt cleanup fails
abhishekraok 120ab9a
Merge remote-tracking branch 'origin/main' into codex/oct31-483
abhishekraok ab60211
Log the background-process change in the result compatibility log
abhishekraok dec5978
Merge remote-tracking branch 'origin/codex/oct31-483' into codex/oct3…
abhishekraok cb5ab0a
Merge remote-tracking branch 'origin/main' into codex/oct31-483-sessions
abhishekraok File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,241 @@ | ||
| """Serialize persistent shell operations without blocking the HTTP server.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import asyncio | ||
| import contextlib | ||
| import os | ||
| import signal | ||
| from collections.abc import Awaitable, Callable | ||
| from typing import Any, TypeVar | ||
|
|
||
| from swerex.exceptions import CommandTimeoutError | ||
| from swerex.runtime import local | ||
| from swerex.runtime.abstract import ( | ||
| BashAction, | ||
| BashInterruptAction, | ||
| BashObservation, | ||
| CloseResponse, | ||
| CloseSessionRequest, | ||
| CloseSessionResponse, | ||
| CreateBashSessionResponse, | ||
| CreateSessionRequest, | ||
| CreateSessionResponse, | ||
| ) | ||
| from swerex.runtime.local import BashSession | ||
|
|
||
| _T = TypeVar("_T") | ||
|
|
||
|
|
||
| class ResponsiveBashSession(BashSession): | ||
| """Keep one pexpect reader per session while other HTTP requests run.""" | ||
|
|
||
| def __init__(self, *args: Any, **kwargs: Any): | ||
| super().__init__(*args, **kwargs) | ||
| self._operation_lock = asyncio.Lock() | ||
| self._active_worker: asyncio.Task[Any] | None = None | ||
| self._closing = False | ||
| self._interrupt_task: asyncio.Task[None] | None = None | ||
|
|
||
| def _kill_foreground(self, *, allow_shell: bool = True) -> None: | ||
| with contextlib.suppress(RuntimeError, OSError): | ||
| foreground = os.tcgetpgrp(self.shell.child_fd) | ||
| if ( | ||
| foreground > 0 | ||
| and foreground != os.getpgrp() | ||
| and (allow_shell or foreground != self.shell.pid) | ||
| ): | ||
| os.killpg(foreground, signal.SIGKILL) | ||
|
|
||
| async def _finish_reader_timeout(self) -> None: | ||
| # Ownership is retained, so a non-shell foreground group still belongs | ||
| # to this action, even if its command advanced to another external job. | ||
| self._kill_foreground(allow_shell=False) | ||
|
|
||
| def drain_prompt() -> None: | ||
| self.shell.expect(self._ps1, timeout=1.0) | ||
|
|
||
| # Another SIGINT could flush the completed job's prompt without creating | ||
| # a replacement when the persistent shell ignores SIGINT. | ||
| try: | ||
| await asyncio.to_thread(drain_prompt) | ||
| except Exception as error: | ||
| self._closing = True | ||
| self._kill_foreground(allow_shell=False) | ||
|
|
||
| def close_terminal() -> None: | ||
| shell = self.shell | ||
| try: | ||
| shell.close(force=True) | ||
| finally: | ||
| if shell.isalive() and not shell.terminate(force=True): | ||
| raise RuntimeError("Session terminal could not be terminated") | ||
|
|
||
| try: | ||
| await asyncio.to_thread(close_terminal) | ||
| except Exception as cleanup_error: | ||
| raise error from cleanup_error | ||
| raise | ||
|
|
||
| async def _interrupt_active(self, task: asyncio.Task[Any]) -> None: | ||
| if self._active_worker is not task: | ||
| return | ||
| if self._interrupt_task is None: | ||
| self._interrupt_task = asyncio.create_task(self._stop_active(task)) | ||
| await asyncio.shield(self._interrupt_task) | ||
|
|
||
| async def _stop_active(self, task: asyncio.Task[Any]) -> None: | ||
| if self._active_worker is not task: | ||
| return | ||
| if task.done(): | ||
| if not task.cancelled() and isinstance(task.exception(), CommandTimeoutError): | ||
| await self._finish_reader_timeout() | ||
| return | ||
| with contextlib.suppress(RuntimeError, OSError): | ||
| self.shell.sendintr() | ||
| try: | ||
| await asyncio.wait_for(asyncio.shield(task), 0.5) | ||
| return | ||
| except CommandTimeoutError: | ||
| await self._finish_reader_timeout() | ||
| return | ||
| except asyncio.TimeoutError: # noqa: UP041 -- remote Python 3.10 | ||
| pass | ||
| except Exception: | ||
| return | ||
| if self._active_worker is not task: | ||
| return | ||
| if task.done(): | ||
| if not task.cancelled() and isinstance(task.exception(), CommandTimeoutError): | ||
| await self._finish_reader_timeout() | ||
| return | ||
| # A foreground job may ignore SIGINT. Signal only this terminal's | ||
| # foreground group, leaving the server and other sessions untouched. | ||
| self._kill_foreground() | ||
| try: | ||
| await asyncio.wait_for(asyncio.shield(task), 1.0) | ||
| return | ||
| except CommandTimeoutError: | ||
| await self._finish_reader_timeout() | ||
| return | ||
| except asyncio.TimeoutError: # noqa: UP041 -- remote Python 3.10 | ||
| pass | ||
| except Exception: | ||
| return | ||
|
|
||
| # Closing an unresponsive terminal wakes its remaining pexpect reader. | ||
| def close_terminal() -> None: | ||
| if self._active_worker is task and not task.done(): | ||
| self.shell.close(force=True) | ||
|
|
||
| await asyncio.to_thread(close_terminal) | ||
| try: | ||
| await asyncio.shield(task) | ||
| except CommandTimeoutError: | ||
| await self._finish_reader_timeout() | ||
| except Exception: | ||
| pass | ||
|
|
||
| async def _run_worker( | ||
| self, operation: Callable[[], Awaitable[_T]], *, allow_closing: bool = False | ||
| ) -> _T: | ||
| async with self._operation_lock: | ||
| if self._closing and not allow_closing: | ||
| raise RuntimeError("Session is closing") | ||
|
|
||
| async def run_operation() -> _T: | ||
| return await operation() | ||
|
|
||
| def run() -> _T: | ||
| return asyncio.run(run_operation()) | ||
|
|
||
| task = asyncio.create_task(asyncio.to_thread(run)) | ||
| self._active_worker = task | ||
| cancelled = False | ||
| while not task.done(): | ||
| try: | ||
| await asyncio.shield(task) | ||
| except asyncio.CancelledError: | ||
| cancelled = True | ||
| # Keep the lock until the sole reader has settled. | ||
| try: | ||
| await self._interrupt_active(task) | ||
| except asyncio.CancelledError: | ||
| continue | ||
| except Exception: | ||
| # Failed cleanup leaves the terminal unusable; the | ||
| # control-task handling below surfaces the error. | ||
| self._closing = True | ||
| break | ||
| except Exception: | ||
| break | ||
| # A reader deadline does not stop its command. Keep ownership until | ||
| # the shared interrupt cleanup and its prompt read have settled. | ||
| control = self._interrupt_task | ||
| if control is not None: | ||
| while not control.done(): | ||
| try: | ||
| await asyncio.shield(control) | ||
| except asyncio.CancelledError: | ||
| cancelled = True | ||
| except Exception: | ||
| break | ||
| error = control.exception() | ||
| if error is not None: | ||
| self._closing = True | ||
| else: | ||
| error = None | ||
| self._interrupt_task = None | ||
| self._active_worker = None | ||
| if cancelled: | ||
| with contextlib.suppress(Exception): | ||
| task.result() | ||
| raise asyncio.CancelledError | ||
| if error is not None: | ||
| raise error | ||
| return task.result() | ||
|
|
||
| async def start(self) -> CreateBashSessionResponse: | ||
| return await self._run_worker(super().start) | ||
|
|
||
| async def run(self, action: BashAction | BashInterruptAction) -> BashObservation: | ||
| if isinstance(action, BashInterruptAction) and self._active_worker is not None: | ||
| await self._interrupt_active(self._active_worker) | ||
| # The existing reader consumed the prompt. Reading it again could | ||
| # block when the shell itself ignores SIGINT. | ||
| return BashObservation(output="", exit_code=0) | ||
| return await self._run_worker(lambda: super(ResponsiveBashSession, self).run(action)) | ||
|
|
||
| async def close(self) -> CloseSessionResponse: | ||
| self._closing = True | ||
| if self._active_worker is not None: | ||
| try: | ||
| await self._interrupt_active(self._active_worker) | ||
| except Exception as exc: | ||
| self.logger.warning("Session interrupt cleanup failed during close: %s", exc) | ||
| return await self._run_worker(super().close, allow_closing=True) | ||
|
|
||
|
|
||
| class ResponsiveSessionRuntime(local.LocalRuntime): | ||
| """Serialize registry mutations across offloaded shell lifecycles.""" | ||
|
|
||
| def __init__(self, **kwargs: Any): | ||
| super().__init__(**kwargs) | ||
| self._lifecycle_lock = asyncio.Lock() | ||
|
|
||
| async def create_session(self, request: CreateSessionRequest) -> CreateSessionResponse: | ||
| async with self._lifecycle_lock: | ||
| return await super().create_session(request) | ||
|
|
||
| async def close_session(self, request: CloseSessionRequest) -> CloseSessionResponse: | ||
| async with self._lifecycle_lock: | ||
| return await super().close_session(request) | ||
|
|
||
| async def close(self) -> CloseResponse: | ||
| async with self._lifecycle_lock: | ||
| return await super().close() | ||
|
|
||
|
|
||
| def install() -> None: | ||
| """Install persistent sessions before the runtime creates any shell.""" | ||
| vars(local).update(BashSession=ResponsiveBashSession, LocalRuntime=ResponsiveSessionRuntime) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.