Skip to content

app-server: box request dispatch future to reduce stack pressure#12421

Merged
charley-oai merged 1 commit intomainfrom
app-server/box-request-dispatch-future-main
Feb 23, 2026
Merged

app-server: box request dispatch future to reduce stack pressure#12421
charley-oai merged 1 commit intomainfrom
app-server/box-request-dispatch-future-main

Conversation

@charley-oai
Copy link
Collaborator

@charley-oai charley-oai commented Feb 21, 2026

Summary

  • box the delegated CodexMessageProcessor::process_request(...) future inside MessageProcessor::process_request
  • reduce stack pressure in the app-server request dispatcher without changing request behavior
  • document/confirm the root cause path for the detached-review stack overflow regression introduced on main by #11786

Root Cause (high confidence)

I bisected main using a deterministic low-stack repro and found the first bad commit:

  • 1f54496c48ffb9678095cc91d40556faa57c99fbapp-server: expose loaded thread status via read/list and notifications (#11786)

Why that commit can trigger overflows:

  • MessageProcessor::process_request is a large async fn that sometimes delegates to the even larger CodexMessageProcessor::process_request.
  • Awaiting that delegated future inline makes the outer MessageProcessor::process_request future type include the full delegated future state machine.
  • #11786 significantly grew the delegated future (new thread-status tracking + additional async work), which increased stack usage for the wrapper future enough to cross the threshold on smaller Tokio worker stacks.
  • This matches the prior LLDB crash frame in message_processor.rs and explains why even initialization-only paths became more stack-sensitive.

Why This Fix

This is a small, behavior-preserving stack-footprint reduction at the source:

  • We heap-box the delegated future (.boxed().await) in the other => branch.
  • That prevents the wrapper future from inlining the full CodexMessageProcessor::process_request future state machine.
  • It reduces per-request stack usage while preserving control flow and request semantics.

This complements (but does not replace) the runtime worker stack-size mitigation already added in codex-arg0.

Validation

Local targeted tests (current branch)

  • just fmt
  • cargo test -p codex-app-server --test all suite::user_agent::get_user_agent_returns_current_codex_user_agent -- --nocapture
  • cargo test -p codex-app-server --test all suite::v2::review::review_start_with_detached_delivery_returns_new_thread_id -- --nocapture

Deterministic before/after repro on the culprit commit (1f54496c48)

I validated the patch directly against the bisected culprit commit in a throwaway worktree.

  1. Detached review low-stack repro (matches reported failure path):
  • Before patch:
    • CARGO_TARGET_DIR=/tmp/codex-bisect-target RUST_MIN_STACK=2031616 cargo test -q -p codex-app-server --test all suite::v2::review::review_start_with_detached_delivery_returns_new_thread_id -- --nocapture
    • Result: tokio-runtime-worker stack overflow (fatal runtime error: stack overflow)
  • After applying this patch:
    • same command
    • Result: test passes (ok)
  1. Initialization-only repro (shows wrapper future stack reduction directly):
  • Before patch on 1f54496c48:
    • run codex-app-server with a minimal initialize request and RUST_MIN_STACK=294912
    • Result: tokio-runtime-worker stack overflow
  • After applying this patch:
    • same command
    • Result: process exits successfully

Bisect result

git bisect (good=e00080cea, bad=d87cf7794) identified:

  • first bad commit: 1f54496c48ffb9678095cc91d40556faa57c99fb

Risk

Low:

  • No protocol/schema changes
  • No request handling behavior changes
  • Only changes how the delegated async future is stored (heap allocation instead of being inlined into the wrapper future state machine)

Codex author

codex resume 019c7e1f-4cd9-72b3-aa74-826e9d38c218

@charley-oai
Copy link
Collaborator Author

@codex review this

@chatgpt-codex-connector
Copy link
Contributor

Codex Review: Didn't find any major issues. Delightful!

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector
Copy link
Contributor

Security review completed. No security issues were found in this pull request.

View security finding report

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

Copy link
Collaborator

@jif-oai jif-oai left a comment

Choose a reason for hiding this comment

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

TLDR: you move the implicit chain from stack to heap. LGTM

@charley-oai charley-oai merged commit 3cea3e6 into main Feb 23, 2026
31 of 33 checks passed
@charley-oai charley-oai deleted the app-server/box-request-dispatch-future-main branch February 23, 2026 18:05
@github-actions github-actions bot locked and limited conversation to collaborators Feb 23, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants