Repository navigation
Conversation
After a gap-fill, '_phase5_consume_segment_and_postprocess' fed the
next queued out-of-order segment back into 'session.tcp_fsm()' from
inside the dispatch that was still processing the current one. Each
queued segment therefore nested one more FSM dispatch (~10 interpreter
frames), so a run of a few hundred OOO segments raised RecursionError
and killed the packet-handler thread. Seen in practice under a
speedtest through a VPN tunnel, where reordering produced long OOO runs.
The drain site now appends the released segment to a per-session
'_ooo_ready' deque, and the outermost 'tcp_fsm()' call processes that
deque in a loop under the (re-entrant) FSM lock once its own dispatch
is done; nested calls see '_ooo_draining' and leave the next segment
to the loop, so the depth stays at two however long the run is.
Segments are still processed in sequence order, each through the full
'tcp_fsm()' path. Nothing in the dispatch ran after the old nested
call ('_process_ack_packet' returns right after it), so the order of
effects is unchanged.
Reference: RFC 9293 §3.10.7.4 (segments with higher beginning sequence
numbers SHOULD be held for later processing, SHLD-31).
New integration test
'test__data_transfer_out_of_order__long_queue_drains_without_recursion'
(300 queued segments, then the gap-fill) raised RecursionError before
the fix and passes after it.
make test: 13845 passing, 11 skipped. make lint clean.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After a gap-fill, '_phase5_consume_segment_and_postprocess' fed the next queued out-of-order segment back into 'session.tcp_fsm()' from inside the dispatch that was still processing the current one. Each queued segment therefore nested one more FSM dispatch (~10 interpreter frames), so a run of a few hundred OOO segments raised RecursionError and killed the packet-handler thread. Seen in practice under a speedtest through a VPN tunnel, where reordering produced long OOO runs.
The drain site now appends the released segment to a per-session '_ooo_ready' deque, and the outermost 'tcp_fsm()' call processes that deque in a loop under the (re-entrant) FSM lock once its own dispatch is done; nested calls see '_ooo_draining' and leave the next segment to the loop, so the depth stays at two however long the run is. Segments are still processed in sequence order, each through the full 'tcp_fsm()' path. Nothing in the dispatch ran after the old nested call ('_process_ack_packet' returns right after it), so the order of effects is unchanged.
Reference: RFC 9293 §3.10.7.4 (segments with higher beginning sequence numbers SHOULD be held for later processing, SHLD-31).
New integration test
'test__data_transfer_out_of_order__long_queue_drains_without_recursion' (300 queued segments, then the gap-fill) raised RecursionError before the fix and passes after it.
make test: 13845 passing, 11 skipped. make lint clean.