Retry on EPROTOTYPE on socket writes. - #1706
Merged
Merged
Conversation
glbrntt
reviewed
Dec 17, 2020
| case (EWOULDBLOCK, true, _): | ||
| return .wouldBlock(0) | ||
| #if os(macOS) || os(iOS) || os(watchOS) || os(tvOS) | ||
| case (EPROTOTYPE, _, eprototypeWorkaround): |
Contributor
There was a problem hiding this comment.
Is this meant to be true rather than eprototypeWorkaround?
Motivation: When writing to a network socket on Apple platforms it is possible to see EPROTOTYPE returned as an error. This is an undocumented and special-case error code that appears to be associated with socket shutdown, and so can fire when writing to a socket that is being shut down by the other side. This should not be fired into the pipeline but instead should be retried. Modifications: - Retry EPROTOTYPE errors on socket write methods. - Add an (unfortunately) probabilistic test bed. Result: Should avoid weird error cases.
Lukasa
force-pushed
the
cb-eprototype
branch
from
December 17, 2020 17:00
ede76d1 to
8ee5bcd
Compare
Lukasa
added a commit
to Lukasa/swift-nio
that referenced
this pull request
Dec 18, 2020
This reverts commit 4853e91. While we have been able to observe the effect that this change was trying to workaround, the change seems to interact poorly with a different issue in Big Sur that can cause EPROTOTYPE to be consistently emitted during socket writes on otherwise connected sockets. This would change a connection-terminating error into a 100% CPU spin that rendered the event loop entirely useless: a substantial regression. For this reason, we should back this out until the issue is better characterised.
Lukasa
added a commit
that referenced
this pull request
Dec 18, 2020
This reverts commit 4853e91. While we have been able to observe the effect that this change was trying to workaround, the change seems to interact poorly with a different issue in Big Sur that can cause EPROTOTYPE to be consistently emitted during socket writes on otherwise connected sockets. This would change a connection-terminating error into a 100% CPU spin that rendered the event loop entirely useless: a substantial regression. For this reason, we should back this out until the issue is better characterised.
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
3 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.
Motivation:
When writing to a network socket on Apple platforms it is possible to
see EPROTOTYPE returned as an error. This is an undocumented and
special-case error code that appears to be associated with socket
shutdown, and so can fire when writing to a socket that is being shut
down by the other side. This should not be fired into the pipeline but
instead should be retried.
Modifications:
Result:
Should avoid weird error cases.
Resolves swift-server/async-http-client#322.