Github user ivmaykov commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/669#discussion_r233649908
--- Diff:
zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java
---
@@ -116,170 +116,104 @@ public void channelConnected(ChannelHandlerContext
ctx,
NettyServerCnxn cnxn = new NettyServerCnxn(channel,
zkServer, NettyServerCnxnFactory.this);
- ctx.setAttachment(cnxn);
+ ctx.channel().attr(CONNECTION_ATTRIBUTE).set(cnxn);
if (secure) {
- SslHandler sslHandler =
ctx.getPipeline().get(SslHandler.class);
- ChannelFuture handshakeFuture = sslHandler.handshake();
+ SslHandler sslHandler =
ctx.pipeline().get(SslHandler.class);
+ Future<Channel> handshakeFuture =
sslHandler.handshakeFuture();
handshakeFuture.addListener(new
CertificateVerifier(sslHandler, cnxn));
} else {
- allChannels.add(ctx.getChannel());
+ allChannels.add(ctx.channel());
addCnxn(cnxn);
}
}
@Override
- public void channelDisconnected(ChannelHandlerContext ctx,
- ChannelStateEvent e) throws Exception
- {
+ public void channelInactive(ChannelHandlerContext ctx) throws
Exception {
if (LOG.isTraceEnabled()) {
- LOG.trace("Channel disconnected " + e);
+ LOG.trace("Channel inactive {}", ctx.channel());
}
- NettyServerCnxn cnxn = (NettyServerCnxn) ctx.getAttachment();
+ allChannels.remove(ctx.channel());
+ NettyServerCnxn cnxn =
ctx.channel().attr(CONNECTION_ATTRIBUTE).getAndSet(null);
if (cnxn != null) {
if (LOG.isTraceEnabled()) {
- LOG.trace("Channel disconnect caused close " + e);
+ LOG.trace("Channel inactive caused close {}", cnxn);
}
cnxn.close();
}
}
@Override
- public void exceptionCaught(ChannelHandlerContext ctx,
ExceptionEvent e)
- throws Exception
- {
- LOG.warn("Exception caught " + e, e.getCause());
- NettyServerCnxn cnxn = (NettyServerCnxn) ctx.getAttachment();
+ public void exceptionCaught(ChannelHandlerContext ctx, Throwable
cause) throws Exception {
--- End diff --
We call `cnxn.close()` at the end of `exceptionCaught()`, which will end up
closing the channel so `channelInactive()` will get called, so I think it would
be redundant to remove from `allChannels` here.
---
