Skip to content

fix(llm): stop Codex process before closing read pipes - #578

Merged
codelion merged 1 commit into
algorithmicsuperintelligence:mainfrom
Elon-Jiang:fix/codex-process-shutdown
Nov 1, 2026
Merged

codelion merged 1 commit into
algorithmicsuperintelligence:mainfrom
Elon-Jiang:fix/codex-process-shutdown

Conversation

@Elon-Jiang

Copy link
Copy Markdown
Contributor

Problem

_AppServerSession.close() closes stdout before terminating its process. If the background reader is blocked reading that pipe and the server remains alive, closing the text stream waits for the reader's lock. Shutdown never reaches terminate() or its timeout/kill fallback.

This also prevents _run_app_server() from returning its request timeout: its finally block gets stuck in session.close(). Reproduced using a real local Python child that announces readiness then sleeps without reading stdin. The sleeping child and blocked reader are always rescued and reaped by the regression test cleanup.

Change

Keep closing stdin first, then terminate and wait for the child, escalating to kill if needed. Wait after kill as well to reap it. Close stdout/stderr only after that process shutdown sequence, so a reader waiting for child output can reach EOF.

Add real-process regressions for direct close, request-timeout cleanup, and a child ignoring SIGTERM. The last case exercises the existing five-second grace period and forced kill/reap on POSIX. The two other cases are not POSIX-specific.

Validation

Python 3.12.13, macOS arm64, main cb49662150aa794149d477379c605a3bff0c08a2:

  • Before: 3 failed, with shutdown unable to finish without test rescue.
  • After: 3 passed in about 5.1 seconds; children are reaped and reader/close threads stop.
  • OPENAI_API_KEY=test-key-for-unit-tests python -m unittest discover tests: 1,006 tests run, 2 skipped; all remaining tests pass.
  • Black and isort checks on both files, plus git diff --check: pass.

Tests launch only the running Python interpreter. No real Codex process, account, model call, or user workspace modification is involved. Native Windows was not run; the SIGTERM-ignore regression skips on non-POSIX systems.

This does not change JSON-RPC buffering/auth/sandbox handling in #558 or text encoding in #577; both patches were checked and do not change shutdown ordering.

Prepared with AI assistance from source review, with actual process and timeout-path verification.

@codelion
codelion merged commit 927ef41 into algorithmicsuperintelligence:main Nov 1, 2026
5 checks passed
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.

2 participants