Fix FIN_WAIT_2 accumulation by draining sockets before close#615
Open
renaudallard wants to merge 1 commit into
Open
Fix FIN_WAIT_2 accumulation by draining sockets before close#615renaudallard wants to merge 1 commit into
renaudallard wants to merge 1 commit into
Conversation
conn_destroy_contents() called close() right after a FIN was sent to the peer, orphaning the socket while it was still in FIN_WAIT_2. Linux reaps such sockets via net.ipv4.tcp_fin_timeout, but OpenBSD has no equivalent, so they pile up until the proxy stalls. Add close_socket(), which sends our FIN with shutdown(SHUT_WR), drains the peer until read() returns 0, then close()s. By then the close handshake is complete and the socket moves to TIME_WAIT, which the kernel reaps on its own. Use it for both descriptors in conn_destroy_contents() and add the matching shutdown(server_fd, SHUT_WR) in relay_connection().
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.
Re-opens #600 with fresh code on top of current master (the original branch was deleted during a repo cleanup). @rofl0r had said he planned to merge it after reading up on the close semantics — this is the same fix, rebased and tidied up.
Problem
On OpenBSD, proxied connections accumulate in FIN_WAIT_2 and are never reaped. Once enough build up, the proxy stalls.
After
relay_connection()sends a FIN withshutdown(SHUT_WR),conn_destroy_contents()callsclose()without waiting for the peer's FIN, orphaning the socket while it is still in FIN_WAIT_2. The idle-timeout and poll-error return paths are worse: they skipshutdown()entirely. On Linux this is masked bynet.ipv4.tcp_fin_timeout(default 60s); OpenBSD has no equivalent, so they persist indefinitely.Why a read after shutdown (re: @rofl0r's question on #600)
It isn't that a read is "required" after shutdown in general — it's the mechanism to detect the peer's FIN so we keep the fd open until the four-way handshake finishes.
shutdown(SHUT_WR)sends our FIN while the fd stays open;read()returning 0 means the peer's FIN arrived and the socket moved to TIME_WAIT. Closing then is harmless because TIME_WAIT is self-limiting (2×MSL). Closing earlier, in FIN_WAIT_2, orphans the socket and leaves its fate to a kernel timeout OpenBSD doesn't have. References: RFC 793 §3.5 and Stevens, UNIX Network Programming Vol. 1 §6.6.Fix
close_socket()insock.c:shutdown(SHUT_WR), drain the peer with a 10sSO_RCVTIMEOuntilread()returns 0, thenclose(). It's self-contained, so it also covers the timeout/poll-error paths that previously skippedshutdown(). The 10s cap means a peer that never sends its FIN can't tie up a worker thread indefinitely.conn_destroy_contents().shutdown(server_fd, SHUT_WR)inrelay_connection(), symmetric with the existing client-side shutdown, so the upstream gets its FIN promptly.Testing
Tested on OpenBSD: connections transition through TIME_WAIT normally instead of accumulating in FIN_WAIT_2. Builds cleanly on Linux with no new warnings.