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
This commit is contained in:
Johannes Weiss 2019-01-07 14:18:48 +00:00 committed by GitHub
parent fa2ebe4a56
commit f38984d80e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 29 additions and 1 deletions

View File

@ -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) {

View File

@ -51,6 +51,7 @@ extension ChannelPipelineTest {
("testRemovingByNameWithFutureNotInChannel", testRemovingByNameWithFutureNotInChannel),
("testRemovingByReferenceWithPromiseStillInChannel", testRemovingByReferenceWithPromiseStillInChannel),
("testRemovingByReferenceWithFutureNotInChannel", testRemovingByReferenceWithFutureNotInChannel),
("testFireChannelReadInInactiveChannelDoesNotCrash", testFireChannelReadInInactiveChannelDoesNotCrash),
]
}
}

View File

@ -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())
}
}