Repository navigation
fix(rpc): hold one heavy slot per HTTP batch - #336
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26b3ec5160
ℹ️ 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".
| tokio::task::spawn_blocking(move || { | ||
| let _guard = guard; | ||
| if name == "abci_query" { |
There was a problem hiding this comment.
Keep the guard alive inside detached blocking work
If an HTTP client disconnects while this spawn_blocking task is running, Hyper drops the request future and therefore the guard held by jsonrpc_handler/call_route, but Tokio's already-started blocking task continues executing. Repeating heavy requests followed by immediate disconnects can consequently run an unbounded number of heavy operations despite max_concurrent_heavy_requests; the previous implementation moved the guard into this closure specifically so cancellation could not release the slot before the blocking work finished.
Useful? React with 👍 / 👎.
Celestia Core v0.42.8 holds one heavy-request semaphore slot for an entire HTTP JSON-RPC batch and keeps it until the buffered response is written. Rust previously released and reacquired the slot for every batch element, allowing concurrent batches to interleave large responses and letting the configured resource bound undercount them.
This change reuses one admission for all heavy elements in a batch and owns that admission in the Hyper response body so completion and cancellation both release it. URI and WebSocket calls keep their existing per-call admission behavior. The RPC qualification ledger is rebound to the changed audited source without changing its scope or readiness claims.
Validation:
cargo test -p rpc --lib(23 passed)cargo clippy -p rpc --all-targets --all-features -- -D warningscargo fmt --all --checkgit diff --check