Permalink
Browse files

remove bogus 'is active' assertion in channelRead0 (#739)

Motivation:

BaseSocketChannel used to assert that `channelRead0` is only called when
the channel is active. That's bogus and it's trivial to hit this issue
when calling `fireChannelRead` in the last `ChannelHandler` when the
channel has gone inactive.

Modifications:

remove assertion, add comment

Result:

fewer crashes
  • Loading branch information...
weissi committed Jan 7, 2019
1 parent fa2ebe4 commit f38984d80efd8548e9df6e3edba6193dbc71be54
@@ -1076,8 +1076,8 @@ class BaseSocketChannel<T: BaseSocket>: SelectableChannel, ChannelCore {
}

public func channelRead0(_ data: NIOAny) {
assert(self.lifecycleManager.isActive)
// Do nothing by default
// note: we can't assert that we're active here as TailChannelHandler will call this on channelRead
}

public func errorCaught0(error: Error) {
@@ -51,6 +51,7 @@ extension ChannelPipelineTest {
("testRemovingByNameWithFutureNotInChannel", testRemovingByNameWithFutureNotInChannel),
("testRemovingByReferenceWithPromiseStillInChannel", testRemovingByReferenceWithPromiseStillInChannel),
("testRemovingByReferenceWithFutureNotInChannel", testRemovingByReferenceWithFutureNotInChannel),
("testFireChannelReadInInactiveChannelDoesNotCrash", testFireChannelReadInInactiveChannelDoesNotCrash),
]
}
}
@@ -902,4 +902,31 @@ class ChannelPipelineTest: XCTestCase {
XCTAssertNil(channel.readOutbound())
XCTAssertNoThrow(try channel.throwIfErrorCaught())
}

func testFireChannelReadInInactiveChannelDoesNotCrash() throws {
class FireWhenInactiveHandler: ChannelInboundHandler {
typealias InboundIn = ()
typealias InboundOut = ()

func channelInactive(ctx: ChannelHandlerContext) {
ctx.fireChannelRead(self.wrapInboundOut(()))
}
}
let handler = FireWhenInactiveHandler()
let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
defer {
XCTAssertNoThrow(try group.syncShutdownGracefully())
}
let server = try assertNoThrowWithValue(ServerBootstrap(group: group).bind(host: "127.0.0.1", port: 0).wait())
defer {
XCTAssertNoThrow(try server.close().wait())
}
let client = try assertNoThrowWithValue(ClientBootstrap(group: group)
.channelInitializer { channel in
channel.pipeline.add(handler: handler)
}
.connect(to: server.localAddress!)
.wait())
XCTAssertNoThrow(try client.close().wait())
}
}

0 comments on commit f38984d

Please sign in to comment.