fix(s2n-quic-transport): release connection flow control credits on stop_sending - #3093
fix(s2n-quic-transport): release connection flow control credits on stop_sending#3093airtrack wants to merge 2 commits into
Conversation
2ffc63b to
2cbcdbd
Compare
|
Hi thanks for the PR. We'll look into this. What was your process for discovering this? |
… data arriving in Stopping state
I have a proxy tool developed based on s2n-quic. The client provides HTTP/SOCKS5 proxy functionality. After the handshake for each TCP proxy connection is completed, it opens an s2n-quic stream to the server. Once the server accepts the stream, it is responsible for connecting to the proxy target address. Then, both the server-side and client-side TCP connections use After running this proxy tool for several days and proxying tens of thousands of TCP connections, the entire s2n-quic connection hangs. The client can no longer receive any data, and the server will not send any data either. I modified the s2n-quic code and added some logs to confirm this issue, but during testing over the past few days, it still gets stuck. I added more logs for further analysis and discovered a second problem. I have also updated the description of this PR. |
|
Minimal independent reproduction (~30s to run): https://gist.github.com/airtrack/7c38b29ae057d9de2ca34e93c0fadb89 Each of the first 8 streams leaks ~512,000 bytes of connection receive window (8 × 512,000 = 4,096,000 > 3,750,000 = InitialMaxData::RECOMMENDED); the 9th stream stalls, and a final fresh-stream check shows client→server still works while server→client is fully stalled — the one-directional, error-free hang seen. |
Release Summary:
fix(s2n-quic-transport): release connection flow control credits on stop_sending
fix(s2n-quic-transport): acquire and release flow control credits for data arriving in Stopping state
Description of changes:
In proxy workloads that use
tokio::io::copy_bidirectionalto shuttle data between a TCP socket and a QUIC stream, the TCP side may close or error before the QUIC receive stream has been fully drained. Whencopy_bidirectionalexits with an error, it drops theReceiveStreamhandle. The Drop path issuesSTOP_SENDINGand resets the receive buffer. But the outstanding connection-level flow control credits —acquiredfromIncomingConnectionFlowControllerminus alreadyreleased— were never returned. These credits cover both unread buffered data and unused window headroom that the stream had reserved. On the peer side, if the send stream was alreadyFinished(all data sent + all ACKed),SendStream::on_stop_sending→init_resetreturnsResetNotNecessaryand sends noRESET_STREAMframe. Without aRESET_STREAMarriving, the localStoppingstate never entersinit_reset()and the credits remain leaked forever.Each leaked stream consumes a portion of the connection receive window. When enough streams leak over days of uptime,
IncomingConnectionFlowController.remaining_window()drops to zero,MAX_DATAframes stop carrying larger values, the peer exhausts itsOutgoingConnectionFlowController, and all streams on the connection hang — new and existing. No errors are reported because the connection and streams remain alive, just starved of flow control.The fix adds two calls after
receive_buffer.reset()in theSTOP_SENDINGbranch ofReceiveStream::poll_request, mirroring the behavior already present in theinit_reset()path:flow_controller.stop_sync()— stops sendingMAX_STREAM_DATAupdates (pointless since we discarded the buffer)flow_controller.release_outstanding_window()— releases allacquired - releasedcredits back toIncomingConnectionFlowController, which triggersMAX_DATAgenerationCall-outs:
Stoppingstate'sdetach()does not setfinal_state_observed = true, so aStoppingstream with noRESET_STREAMresponse stays retained forever. This is a secondary issue not addressed here — cleaning up theStopping →terminal state transition is left for follow-up.FinishedwhenSTOP_SENDINGarrives. In proxy workloads where many streams complete normally, this occurs frequently enough to exhaust the window over days.Testing:
stop_sending_releases_outstanding_connection_flow_control_creditsfeeds 2000 bytes, does not consume any, callsstop_sending(simulating the Drop path), and assertsremaining_window == desired_connection_flow_control_window. Without the fix,remaining_window == desired - 2000(leaked); with the fix, fully recovered.s2n-quic-transporttests pass with zero regressions.Fix 2: Data arriving in Stopping state leaks connection flow control credits
Description of changes:
After the first fix,
STOP_SENDINGcorrectly releases credits for data already buffered in the receive buffer. However, there is a race: the peer may send additionalSTREAMframes after the local side issuesSTOP_SENDINGbut before the peer receives and processes it. These frames arrive while the stream is already inStoppingstate, andon_dataforStoppingonly tracksmissing_datafor FIN detection — it never callsacquire_window_up_to, so the connection flow control window is never charged for these bytes.From the peer's perspective, the send credit was consumed (
bytes_sentincreased). From the receiver's perspective, the data was discarded without accounting. This desynchronises the connection flow control window. Over time, the cumulative gap preventsMAX_DATAfrom advancing, and the connection deadlocks.The fix follows the same pattern as
init_reset(): acquire flow control credits up to the data offset (per RFC 9000 §4.5), then immediately release all outstanding credits since the data will not be consumed by the application.s2n-quic-transport/src/stream/receive_stream.rs—on_dataStopping branch:acquire_window_up_to(data_end)whenfinal_sizeis unknown — validates and accounts for the data in the connection flow controllerrelease_outstanding_window()— releases the newly acquired credits immediatelyTesting:
stop_sending_releases_credits_for_data_arriving_in_stopping_state: feeds 2000 bytes, callsstop_sending(simulating Drop), then feeds another 1000 bytes while in Stopping state. Assertsconsumed_windowincreases by 1000 andremaining_windowstays atdesired_connection_flow_control_window. Without the fix,consumed_windowstays at 2000 (the new data was silently discarded).s2n-quic-transporttests pass with zero regressions.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.