Better handle ECONNRESET on connected datagram sockets. #2979
Conversation
This file contains 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
Motivation: When a connected datagram socket sends a datagram to a port or host that is not listening, an ICMP Destination Unreachable message may be returned. That message triggers an ECONNRESET to be produced at the socket layer. On Darwin we handled this well, but on Linux it turned out that this would push us into connection teardown and, eventually, into a crash. Not so good. To better handle this, we need to distinguish EPOLLERR from EPOLLHUP on datagram sockets. In these cases, we should check whether the socket error was fatal and, if it was not, we should continue our execution having fired the error down the pipeline. Modifications: Modify the selector code to distinguish reset and error. Add support for our channels to handle errors. Have most channels handle errors as resets. Override the logic for datagram channels to duplicate the logic in readable. Add a unit test. Result: Better datagrams for all.
a0976f9
to
56e762a
Compare
FranzBusch
approved these changes
Nov 20, 2024
Comment on lines
+1762
to
+1767
| // Write again. | ||
| XCTAssertNoThrow(try self.firstChannel.writeAndFlush(writeData).wait()) | ||
|
|
||
| // This should trigger an error. | ||
| let errors = try self.firstChannel.waitForErrors(count: 1) | ||
| XCTAssertEqual((errors[0] as? IOError)?.errnoCode, ECONNREFUSED) |
There was a problem hiding this comment.
This is really interesting. So the write succeeds since we send out the data on the socket however when the subsequently get the ECONNREFUSED since there was no reachable destination for the packet.
glbrntt
approved these changes
Nov 20, 2024
Lukasa
added a commit
to Lukasa/swift-nio
that referenced
this pull request
Nov 22, 2024
Motivation: When a connected datagram socket sends a datagram to a port or host that is not listening, an ICMP Destination Unreachable message may be returned. That message triggers an ECONNRESET to be produced at the socket layer. On Darwin we handled this well, but on Linux it turned out that this would push us into connection teardown and, eventually, into a crash. Not so good. To better handle this, we need to distinguish EPOLLERR from EPOLLHUP on datagram sockets. In these cases, we should check whether the socket error was fatal and, if it was not, we should continue our execution having fired the error down the pipeline. Modifications: Modify the selector code to distinguish reset and error. Add support for our channels to handle errors. Have most channels handle errors as resets. Override the logic for datagram channels to duplicate the logic in readable. Add a unit test. Result: Better datagrams for all. (cherry picked from commit 2a3a333)
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.
Motivation:
When a connected datagram socket sends a datagram to a port or host that is not listening, an ICMP Destination Unreachable message may be returned. That message triggers an ECONNRESET to be produced at the socket layer.
On Darwin we handled this well, but on Linux it turned out that this would push us into connection teardown and, eventually, into a crash. Not so good.
To better handle this, we need to distinguish EPOLLERR from EPOLLHUP on datagram sockets. In these cases, we should check whether the socket error was fatal and, if it was not, we should continue our execution having fired the error down the pipeline.
Modifications:
Modify the selector code to distinguish reset and error. Add support for our channels to handle errors.
Have most channels handle errors as resets.
Override the logic for datagram channels to duplicate the logic in readable.
Add a unit test.
Result:
Better datagrams for all.