Skip to content

Commit

Permalink
Ensure channels don't get stuck completely unregistered. (#104)
Browse files Browse the repository at this point in the history
Motivation:

It should be possible to have channels that are not registered
for any form of I/O without them getting stuck in that model
forever.

Modifications:

Remove the code that prevents channels registered for `.none` from
registering for anything else.

Result:

Channels can actually be registered for nothing without becoming
wedged open forever.
  • Loading branch information
Lukasa authored and normanmaurer committed Mar 5, 2018
1 parent 784c5a7 commit 1fdee50
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 1 deletion.
2 changes: 1 addition & 1 deletion Sources/NIO/SocketChannel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ class BaseSocketChannel<T: BaseSocket>: SelectableChannel, ChannelCore {
interestedEvent = .none
return
}
if interested == interestedEvent || interestedEvent == .none {
if interested == interestedEvent {
// we don't need to update and so cause a syscall if we already are registered with the correct event
return
}
Expand Down
1 change: 1 addition & 0 deletions Tests/NIOTests/ChannelTests+XCTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ extension ChannelTests {
("testWeDontCrashIfChannelReleasesBeforePipeline", testWeDontCrashIfChannelReleasesBeforePipeline),
("testAskForLocalAndRemoteAddressesAfterChannelIsClosed", testAskForLocalAndRemoteAddressesAfterChannelIsClosed),
("testReceiveAddressAfterAccept", testReceiveAddressAfterAccept),
("testWeDontJamSocketsInANoIOState", testWeDontJamSocketsInANoIOState),
]
}
}
Expand Down
84 changes: 84 additions & 0 deletions Tests/NIOTests/ChannelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1442,4 +1442,88 @@ public class ChannelTests: XCTestCase {
try clientChannel.closeFuture.wait()
try serverChannel.syncCloseAcceptingAlreadyClosed()
}

func testWeDontJamSocketsInANoIOState() throws {
final class ReadDelayer: ChannelDuplexHandler {
typealias InboundIn = Any
typealias InboundOut = Any
typealias OutboundIn = Any
typealias OutboundOut = Any

public var reads = 0
private var ctx: ChannelHandlerContext!
private var readCountPromise: EventLoopPromise<Void>!
private var waitingForReadPromise: EventLoopPromise<Void>?

func handlerAdded(ctx: ChannelHandlerContext) {
self.ctx = ctx
self.readCountPromise = ctx.eventLoop.newPromise()
}

public func expectRead(loop: EventLoop) -> EventLoopFuture<Void> {
return loop.submit {
self.waitingForReadPromise = loop.newPromise()
}.then { (_: Void) in
return self.waitingForReadPromise!.futureResult
}
}

func channelReadComplete(ctx: ChannelHandlerContext) {
self.waitingForReadPromise?.succeed(result: ())
self.waitingForReadPromise = nil
}

func read(ctx: ChannelHandlerContext) {
self.reads += 1

// Allow the first read through.
if self.reads == 1 {
self.ctx.read()
}
}

public func issueDelayedRead() {
self.ctx.read()
}
}

let group = MultiThreadedEventLoopGroup(numThreads: 1)
defer {
XCTAssertNoThrow(try group.syncShutdownGracefully())
}
let readDelayer = ReadDelayer()

let serverChannel = try ServerBootstrap(group: group)
.serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
.childChannelInitializer {
$0.pipeline.add(handler: readDelayer)
}
.bind(host: "127.0.0.1", port: 0).wait()

let clientChannel = try ClientBootstrap(group: group)
.connect(to: serverChannel.localAddress!).wait()

// We send a first write and expect it to arrive.
var buffer = clientChannel.allocator.buffer(capacity: 12)
let firstReadPromise = readDelayer.expectRead(loop: serverChannel.eventLoop)
buffer.write(staticString: "hello, world")
XCTAssertNoThrow(try clientChannel.writeAndFlush(buffer).wait())
XCTAssertNoThrow(try firstReadPromise.wait())

// We send a second write. This won't arrive immediately.
XCTAssertNoThrow(try clientChannel.writeAndFlush(buffer).wait())
let readFuture = readDelayer.expectRead(loop: serverChannel.eventLoop)
try serverChannel.eventLoop.scheduleTask(in: .milliseconds(100)) {
XCTAssertFalse(readFuture.fulfilled)
}.futureResult.wait()

// Ok, now let it proceed.
try serverChannel.eventLoop.submit {
XCTAssertEqual(readDelayer.reads, 2)
readDelayer.issueDelayedRead()
}.wait()

// The read should go through.
XCTAssertNoThrow(try readFuture.wait())
}
}

0 comments on commit 1fdee50

Please sign in to comment.