Own MCP response work shutdown - #141
Conversation
|
You have reached your Codex usage limits for security reviews. Please try again later. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a36671a18
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2744bc3d5c
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84e3f6c020
ℹ️ 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".
| } else if heartbeatPending { | ||
| heartbeatPending = false | ||
| immediate = .heartbeat | ||
| } else if let pendingBody { |
There was a problem hiding this comment.
When a heartbeat write takes at least one heartbeat interval (for example, with a backpressured client), the timer sets heartbeatPending while a source body is already pending; this ordering then selects another heartbeat first, allowing the timer to re-arm on every slow write and starving the actual MCP response body indefinitely. Check pendingBody before heartbeatPending so keep-alives cannot overtake application data.
Useful? React with 👍 / 👎.
| guard let admittedRequest = connection.admitRequest() else { | ||
| context.close(promise: nil) | ||
| return | ||
| } |
There was a problem hiding this comment.
When a client pipelines another request after one carrying Connection: close (or an HTTP/1.0 non-keepalive request), every subsequent .head is still admitted here and its handler can execute handleTrackedHTTPRequest before the first response reaches the later closeAfterResponse() call. Thus a side-effecting MCP request can run even though its response is subsequently cancelled when the connection closes. The fresh evidence beyond the earlier physical-close issue is this new head-time concurrent admission; close admission as soon as the non-keepalive head is accepted while still deferring the physical close until its response ends.
Useful? React with 👍 / 👎.
| let lease = operation.makeLease() | ||
| requests[operation.id] = operation | ||
| requests.append(operation) | ||
| lock.unlock() |
There was a problem hiding this comment.
When the first response is slow or an open SSE stream, a client can pipeline arbitrarily many small requests and every one is appended here while channelReadComplete continues issuing context.read(); each admission also creates a task/body receipt, and completed handlers retain their prepared response while awaiting the FIFO writer turn. Unlike the previously fixed response-source buffering, this request-level queue remains unbounded, so a single connection can drive memory and task counts until the server is exhausted. Cap outstanding requests or stop reading from the channel until queue capacity becomes available.
Useful? React with 👍 / 👎.
1 participant
Purpose
Prevent an MCP listener generation from shutting down its SwiftNIO event loop while admitted connection, request, or response work is still active, without violating HTTP response ordering or connection lifetime semantics.
Changes
Expect: 100-continuethrough the existing connection FIFO: HTTP/1.1 receives a head-only interim response before its body, HTTP/1.0 ignores the expectation, and unsupported expectations receive an owned 417 close.handleHTTPRequeststream completion tracking for finite completion and cancelled open streams.Connection: closerequests after the acknowledged response end, while retaining owner state through physical channel-close acknowledgement.Testing
swift test --build-system swiftbuild --no-parallelxcodebuild test(15 tests)Screenshots
Not applicable.