Release the upstream socket once a TCP session is finished - #941
Closed
kasnder wants to merge 1 commit into
Closed
Conversation
Since the half-close work, an upstream socket is kept open after the peer's EOF so the client's remaining data can still be written to it. Nothing closes it again once both directions are done: after the client FIN has been consumed (write half shut down, queue empty) and the peer has sent EOF, the descriptor stays registered in the epoll set while the session waits in LAST_ACK for the client's final ACK. A fully closed socket reports EPOLLHUP, which epoll delivers whether or not it was requested, so every epoll_wait() returns immediately and the event loop spins - normally only for the round trip to the app, but for the full TCP_CLOSE_TIMEOUT when the client never acknowledges the FIN. Close the descriptor as soon as neither direction can carry another byte; the session itself still lives on until check_tcp_session() accounts and reaps it. Also skip the IPv6 opener assertions when the host cannot create an AF_INET6 socket at all, so the suite runs on containers without IPv6. Claude-Session: https://claude.ai/code/session_01YRjEng6MN2MmtMDMjEdNRg
kasnder
marked this pull request as ready for review
September 9, 2026 07:32
kasnder
added a commit
that referenced
this pull request
Sep 9, 2026
* Release finished TCP sockets without resetting retransmitted FINs Include the socket-release fix from #941 and retain normal FIN/ACK handling for already-consumed retransmitted payload after the upstream descriptor is released. New payload and conflicting FIN sequences remain rejected. Exercise both close orderings and delayed final ACKs with real epoll, plus queued data, sequence wraparound and duplicate payload/FIN regressions. Validated the native suites on an isolated Android emulator with UBSan; the epoll regression fails before #941 and the retransmission regression fails with #941 alone. * Qualify TUN recovery across one uninterrupted failure run Use the existing total-minus-streak identity to detect successful writes between connectivity polls. Separate failure bursts start a fresh persistence window instead of falsely triggering tunnel recovery. Sustained failures still qualify even when handshakes and receive counters advance. Cover hidden resets, growing bursts, resumed persistent failure and invalid samples. All 40 connectivity checker and monitor tests pass.
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
2 participants
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.
Follow-up from a review of the recent hot-path fixes (#909, #910, #913, #915, #916, #919, #921, #922, #924, #925, #926).
The bug
Since the half-close work, the upstream socket is deliberately kept open past the peer's EOF so the client's remaining data can still be written to it. Nothing closes it again once both directions are finished: after the client FIN has been consumed (write half shut down, forward queue empty) and the peer has sent EOF, the descriptor stays registered in the epoll set while the session waits in
TCP_LAST_ACKfor the client's final ACK.A fully closed socket reports
EPOLLHUP, and epoll delivers that whether or not it was requested — soepoll_wait()returns immediately on every iteration andhandle_events()re-walks the whole session list instead of sleeping. In the normal case that lasts only the round trip to the app, but a client that never acknowledges the FIN (a killed app, a lost ACK) keeps the loop spinning for the fullTCP_CLOSE_TIMEOUT.Before #915 this could not happen: the EOF path closed the socket outright.
The fix
release_finished_upstream()closes the descriptor as soon as neither direction can carry another byte —upstream_read_eof && client_fin_consumed && forward == NULL— which removes it from the epoll set. The session itself is untouched and still lives on untilcheck_tcp_session()accounts and reaps it;monitor_tcp_session()and theTCP_CLOSINGpath already skip a session whose socket is-1.Tests
test_finished_session_releases_upstream_socket()intcp_half_close_test.c: fails on the parent commit, passes here.tcp_queue,tcp_window,tcp_half_close,tcp_epoll,route_flow,udp_state,wg_flow_cacheanddns_frame— all pass.tcp_half_close_test.candtcp_defensive_test.cnow skip when the host cannot create anAF_INET6socket at all, so the suite runs on containers without IPv6 (it aborted there before, which is how this was found).https://claude.ai/code/session_01YRjEng6MN2MmtMDMjEdNRg
Generated by Claude Code