Fix: [Network] connections can use an invalid socket due to a race condition #9731
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.
Motivation / Problem
Fixes #9730.
Description
As by commit message:
Bit difficult story:
The code never assumed this situation to happen. As such, it didn't check if any of the remaining connections was actually connected, but instead assumed at least one of the remaining was. This in result means that
connected_socket
becameINVALID_SOCKET
and things went from bad to worse from there.So now instead, check if any non-error'd socket is in writable state. If so, only than safely assume one of the sockets is connected.
The
if (this->sockets.empty()) {
meant this worked for most people, as it is rare two connections are actually pending, and the first reporting an error before the second manages to connect. Just when timing was bad, it broke.(Normally IPv6 just times out, which takes a few seconds. The IPv4 attempt will succeed well before that, and that case would work perfectly fine).
The
if (this->sockets.empty()) {
can safely be removed, as the start of the function does exactly this already. So nextCheckActivity
the exact same code is executed and starts another connection attempt if needed. Less code duplication is beter :)Limitations
None
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.