-
Notifications
You must be signed in to change notification settings - Fork 532
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set SO_REUSEADDR on epoll tcp listener sockets #4544
Conversation
Unix is a bit more strict about TIME_WAIT state, and actually puts any sockets that have had a valid accept() called on them into the TIME_WAIT state. This makes writing a listener app difficult, as if that ever crashes the bind() will fail for the next few minutes. Pretty much all other TCP libraries set SO_REUSEADDR (Including libuv, which is what our app has used before). Libuv sets it on all TCP sockets, but its generally less required on client sockets, as they rarely actually specify a local port.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4544 +/- ##
==========================================
+ Coverage 86.37% 86.67% +0.29%
==========================================
Files 56 56
Lines 15537 17354 +1817
==========================================
+ Hits 13420 15041 +1621
- Misses 2117 2313 +196 ☔ View full report in Codecov by Sentry. |
} else if (SocketType == CXPLAT_SOCKET_TCP_LISTENER) { | ||
// | ||
// Set SO_REUSEADDR on listener sockets to avoid | ||
// TIME_WAIT state on shutdown. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a bit confused. The listener socket itself can go from listen state to timewait state? I thought a socket had to go through finwait1 and finwait2 to enter timewait state (which implicitly means only sockets who initiate graceful shutdown).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, on linux a listener socket itself can go into timewait. Its a bit weird. But it was extremely easy to reproduce.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mind sharing how it could be repro'd?
Just create a listener socket and have a connection accepted and kill both the accepted connection and the listener socket itself? Then bind to the same listening port will fail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup. Have the listener socket accept a connection, and then kill the listener side. The listener side then will not be able to restart due to a bind failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't the listener socket follow the left side of that tree down to TIME_WAIT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The socket that's accepted from the listener socket would follow the left side but the listener socket itself, in my understanding, would not. When you call accept() on a listener socket, you get a new socket (in SynRcvd or ESTAB state) and the listener itself should stay in listening state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was a bit confusing to me too. libuv actually sets SO_REUSEADDR for both Client initiated sockets and listener sockets unconditionally, but at least for my use case I didn't need it set on the client side, and thats a little more invasive too.
Description
Unix is a bit more strict about TIME_WAIT state, and actually puts any sockets that have had a valid accept() called on them into the TIME_WAIT state.
This makes writing a listener app difficult, as if that ever crashes the bind() will fail for the next few minutes.
Pretty much all other TCP libraries set SO_REUSEADDR (Including libuv, which is what our app has used before). Libuv sets it on all TCP sockets, but its generally less required on client sockets, as they rarely actually specify a local port.
Describe the purpose of and changes within this Pull Request.
Testing
Not really possible to test with our current setup, as a crash is the only way to get a listener socket into TIME_WAIT. A properly shutdown socket doesn't stay in TIME_WAIT.
Documentation
N/A