kezhuw commented on code in PR #1917:
URL: https://github.com/apache/zookeeper/pull/1917#discussion_r1731289663
##########
zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java:
##########
@@ -142,19 +142,30 @@ void connect(InetSocketAddress addr) throws IOException {
connectFuture.addListener(new ChannelFutureListener() {
@Override
public void operationComplete(ChannelFuture channelFuture)
throws Exception {
- // this lock guarantees that channel won't be assigned
after cleanup().
+ boolean lockAcquired = false;
boolean connected = false;
- connectLock.lock();
+
try {
if (!channelFuture.isSuccess()) {
LOG.warn("future isn't success.",
channelFuture.cause());
return;
- } else if (connectFuture == null) {
Review Comment:
How about simple `channelFuture != connectFuture` in earlier stage ? This
should cover `connectFuture == null`. In case of `channelFuture !=
connectFuture` we should do nothing but simple return.
##########
zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java:
##########
@@ -196,13 +209,26 @@ public void operationComplete(ChannelFuture
channelFuture) throws Exception {
}
}
+ boolean cancelled(ChannelFuture channelFuture) {
+ if (connectFuture == null) {
+ LOG.info("connect attempt cancelled");
+ // If the connect attempt was cancelled but succeeded
+ // anyway, make sure to close the channel, otherwise
+ // we may leak a file descriptor.
+ channelFuture.channel().close();
+ return true;
+ }
+ return false;
+ }
+
@Override
void cleanup() {
connectLock.lock();
try {
if (connectFuture != null) {
connectFuture.cancel(false);
connectFuture = null;
+ afterConnectFutureCancel();
}
if (channel != null) {
channel.close().syncUninterruptibly();
Review Comment:
How about drop `.syncUninterruptibly()` part ? This way we will not wait for
channel close completion callback which could deadlock with old
`connectFuture`'s completion callback.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
