Skip to content

Commit 4853e91

Browse files
authored
Retry on EPROTOTYPE on socket writes. (apple#1706)
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.
1 parent 43931b7 commit 4853e91

3 files changed

Lines changed: 95 additions & 6 deletions

File tree

Sources/NIO/System.swift

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -150,18 +150,26 @@ private func preconditionIsNotUnacceptableErrno(err: CInt, where function: Strin
150150
@inline(__always)
151151
@discardableResult
152152
internal func syscall<T: FixedWidthInteger>(blocking: Bool,
153+
eprototypeWorkaround: Bool = false,
153154
where function: String = #function,
154155
_ body: () throws -> T)
155156
throws -> IOResult<T> {
156157
while true {
157158
let res = try body()
158159
if res == -1 {
159160
let err = errno
160-
switch (err, blocking) {
161-
case (EINTR, _):
161+
switch (err, blocking, eprototypeWorkaround) {
162+
case (EINTR, _, _):
162163
continue
163-
case (EWOULDBLOCK, true):
164+
case (EWOULDBLOCK, true, _):
164165
return .wouldBlock(0)
166+
#if os(macOS) || os(iOS) || os(watchOS) || os(tvOS)
167+
case (EPROTOTYPE, _, true):
168+
// EPROTOTYPE can, on Darwin platforms, sometimes fire due to a race in the XNU kernel.
169+
// The socket in question is about to shut down, so we can just retry the syscall and get
170+
// the actual error (usually, but not necessarily, EPIPE).
171+
continue
172+
#endif
165173
default:
166174
preconditionIsNotUnacceptableErrno(err: err, where: function)
167175
throw IOError(errnoCode: err, reason: function)
@@ -356,7 +364,7 @@ internal enum Posix {
356364

357365
@inline(never)
358366
public static func write(descriptor: CInt, pointer: UnsafeRawPointer, size: Int) throws -> IOResult<Int> {
359-
return try syscall(blocking: true) {
367+
return try syscall(blocking: true, eprototypeWorkaround: true) {
360368
sysWrite(descriptor, pointer, size)
361369
}
362370
}
@@ -371,7 +379,7 @@ internal enum Posix {
371379
#if !os(Windows)
372380
@inline(never)
373381
public static func writev(descriptor: CInt, iovecs: UnsafeBufferPointer<IOVector>) throws -> IOResult<Int> {
374-
return try syscall(blocking: true) {
382+
return try syscall(blocking: true, eprototypeWorkaround: true) {
375383
sysWritev(descriptor, iovecs.baseAddress!, CInt(iovecs.count))
376384
}
377385
}
@@ -445,7 +453,7 @@ internal enum Posix {
445453
public static func sendfile(descriptor: CInt, fd: CInt, offset: off_t, count: size_t) throws -> IOResult<Int> {
446454
var written: off_t = 0
447455
do {
448-
_ = try syscall(blocking: false) { () -> ssize_t in
456+
_ = try syscall(blocking: false, eprototypeWorkaround: true) { () -> ssize_t in
449457
#if os(macOS) || os(iOS) || os(watchOS) || os(tvOS)
450458
var w: off_t = off_t(count)
451459
let result: CInt = Darwin.sendfile(fd, descriptor, offset, &w, nil, 0)

Tests/NIOTests/ChannelTests+XCTest.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ extension ChannelTests {
8484
("testFixedSizeRecvByteBufferAllocatorSizeIsConstant", testFixedSizeRecvByteBufferAllocatorSizeIsConstant),
8585
("testCloseInConnectPromise", testCloseInConnectPromise),
8686
("testWritabilityChangeDuringReentrantFlushNow", testWritabilityChangeDuringReentrantFlushNow),
87+
("testTriggerEPROTOTYPE", testTriggerEPROTOTYPE),
8788
]
8889
}
8990
}

Tests/NIOTests/ChannelTests.swift

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2814,6 +2814,40 @@ public final class ChannelTests: XCTestCase {
28142814
XCTAssertNoThrow(try handler.becameUnwritable.futureResult.wait())
28152815
XCTAssertNoThrow(try handler.becameWritable.futureResult.wait())
28162816
}
2817+
2818+
func testTriggerEPROTOTYPE() throws {
2819+
// This is a probabilistic test for https://github.com/swift-server/async-http-client/issues/322.
2820+
// We believe we'll see EPROTOTYPE on write syscalls if we write while the connections are being torn down.
2821+
// To check this we create 500 connections and close them, while the server attempts to write AS FAST AS IT CAN.
2822+
// As this test is probabilistic, we must not ignore transient failures in it.
2823+
let group = MultiThreadedEventLoopGroup(numberOfThreads: 2)
2824+
defer {
2825+
XCTAssertNoThrow(try group.syncShutdownGracefully())
2826+
}
2827+
2828+
let serverLoop = group.next()
2829+
let clientLoop = group.next()
2830+
XCTAssertFalse(serverLoop === clientLoop)
2831+
2832+
let serverFuture = ServerBootstrap(group: serverLoop)
2833+
.childChannelInitializer { channel in
2834+
return channel.pipeline.addHandler(AlwaysBeWritingHandler(vectorWrites: [true, false].randomElement()!))
2835+
}
2836+
.bind(host: "localhost", port: 0)
2837+
2838+
let server: Channel = try assertNoThrowWithValue(try serverFuture.wait())
2839+
defer {
2840+
XCTAssertNoThrow(try server.close().wait())
2841+
}
2842+
2843+
let clientFactory = ClientBootstrap(group: clientLoop)
2844+
let serverAddress = server.localAddress!
2845+
2846+
for _ in 0..<500 {
2847+
let client = try assertNoThrowWithValue(clientFactory.connect(to: serverAddress).wait())
2848+
XCTAssertNoThrow(try client.close().wait())
2849+
}
2850+
}
28172851
}
28182852

28192853
fileprivate final class FailRegistrationAndDelayCloseHandler: ChannelOutboundHandler {
@@ -2926,3 +2960,49 @@ final class ReentrantWritabilityChangingHandler: ChannelInboundHandler {
29262960
}
29272961
}
29282962
}
2963+
2964+
final class AlwaysBeWritingHandler: ChannelInboundHandler {
2965+
typealias InboundIn = ByteBuffer
2966+
typealias OutboundOut = ByteBuffer
2967+
2968+
static let buffer = ByteBuffer(string: "This is some data that I'm sending right now")
2969+
2970+
private let doVectorWrite: Bool
2971+
2972+
init(vectorWrites: Bool) {
2973+
self.doVectorWrite = vectorWrites
2974+
}
2975+
2976+
func channelActive(context: ChannelHandlerContext) {
2977+
self.keepWriting(context: context)
2978+
}
2979+
2980+
func errorCaught(context: ChannelHandlerContext, error: Error) {
2981+
if let error = error as? IOError, error.errnoCode == EPROTOTYPE {
2982+
XCTFail("Received EPROTOTYPE error")
2983+
}
2984+
}
2985+
2986+
private func keepWriting(context: ChannelHandlerContext) {
2987+
if self.doVectorWrite {
2988+
context.write(self.wrapOutboundOut(AlwaysBeWritingHandler.buffer)).whenFailure { error in
2989+
if let error = error as? IOError, error.errnoCode == EPROTOTYPE {
2990+
XCTFail("Received EPROTOTYPE error")
2991+
}
2992+
}
2993+
}
2994+
context.writeAndFlush(self.wrapOutboundOut(AlwaysBeWritingHandler.buffer)).whenComplete { result in
2995+
switch result {
2996+
case .success:
2997+
// We unroll the stack here to avoid blowing it apart.
2998+
context.eventLoop.execute {
2999+
self.keepWriting(context: context)
3000+
}
3001+
case .failure(let error):
3002+
if let error = error as? IOError, error.errnoCode == EPROTOTYPE {
3003+
XCTFail("Received EPROTOTYPE error")
3004+
}
3005+
}
3006+
}
3007+
}
3008+
}

0 commit comments

Comments
 (0)