normanmaurer commented on a change in pull request #753: ZOOKEEPER-3204: 
Reconfig tests are constantly failing on 3.5 after applying Java 11 fix
URL: https://github.com/apache/zookeeper/pull/753#discussion_r254343264
 
 

 ##########
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java
 ##########
 @@ -104,71 +106,102 @@
     boolean isConnected() {
         // Assuming that isConnected() is only used to initiate connection,
         // not used by some other connection status judgement.
-        return channel != null;
+        connectLock.lock();
+        try {
+            return channel != null || connectFuture != null;
+        } finally {
+            connectLock.unlock();
+        }
+    }
+
+    private Bootstrap configureBootstrapAllocator(Bootstrap bootstrap) {
+        ByteBufAllocator testAllocator = TEST_ALLOCATOR.get();
+        if (testAllocator != null) {
+            return bootstrap.option(ChannelOption.ALLOCATOR, testAllocator);
+        } else {
+            return bootstrap;
+        }
     }
 
     @Override
     void connect(InetSocketAddress addr) throws IOException {
         firstConnect = new CountDownLatch(1);
 
-        ClientBootstrap bootstrap = new ClientBootstrap(channelFactory);
-
-        bootstrap.setPipelineFactory(new 
ZKClientPipelineFactory(addr.getHostString(), addr.getPort()));
-        bootstrap.setOption("soLinger", -1);
-        bootstrap.setOption("tcpNoDelay", true);
-
-        connectFuture = bootstrap.connect(addr);
-        connectFuture.addListener(new ChannelFutureListener() {
-            @Override
-            public void operationComplete(ChannelFuture channelFuture) throws 
Exception {
-                // this lock guarantees that channel won't be assgined after 
cleanup().
-                connectLock.lock();
-                try {
-                    if (!channelFuture.isSuccess() || connectFuture == null) {
-                        LOG.info("future isn't success, cause: {}", 
channelFuture.getCause());
-                        return;
-                    }
-                    // setup channel, variables, connection, etc.
-                    channel = channelFuture.getChannel();
-
-                    disconnected.set(false);
-                    initialized = false;
-                    lenBuffer.clear();
-                    incomingBuffer = lenBuffer;
-
-                    sendThread.primeConnection();
-                    updateNow();
-                    updateLastSendAndHeard();
-
-                    if (sendThread.tunnelAuthInProgress()) {
-                        waitSasl.drainPermits();
-                        needSasl.set(true);
-                        sendPrimePacket();
-                    } else {
-                        needSasl.set(false);
-                    }
+        Bootstrap bootstrap = new Bootstrap()
+                .group(eventLoopGroup)
+                .channel(NettyUtils.nioOrEpollSocketChannel())
+                .option(ChannelOption.SO_LINGER, -1)
+                .option(ChannelOption.TCP_NODELAY, true)
+                .handler(new ZKClientPipelineFactory(addr.getHostString(), 
addr.getPort()));
+        bootstrap = configureBootstrapAllocator(bootstrap);
+        bootstrap.validate();
 
-                    // we need to wake up on first connect to avoid timeout.
-                    wakeupCnxn();
-                    firstConnect.countDown();
-                    LOG.info("channel is connected: {}", 
channelFuture.getChannel());
-                } finally {
-                    connectLock.unlock();
+        connectLock.lock();
+        try {
+            connectFuture = bootstrap.connect(addr);
+            connectFuture.addListener(new ChannelFutureListener() {
+                @Override
+                public void operationComplete(ChannelFuture channelFuture) 
throws Exception {
+                    // this lock guarantees that channel won't be assigned 
after cleanup().
+                    connectLock.lock();
+                    try {
+                        if (!channelFuture.isSuccess()) {
+                            LOG.info("future isn't success, cause:", 
channelFuture.cause());
+                            return;
+                        } else 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;
+                        }
+                        // setup channel, variables, connection, etc.
+                        channel = channelFuture.channel();
+
+                        disconnected.set(false);
+                        initialized = false;
+                        lenBuffer.clear();
+                        incomingBuffer = lenBuffer;
 
 Review comment:
   ok 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to