ZOOKEEPER-1839. Deadlock in NettyServerCnxn (Rakesh R via michim)

git-svn-id: https://svn.apache.org/repos/asf/zookeeper/trunk@1551111 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -496,6 +496,8 @@ BUGFIXES:
   ZOOKEEPER-1382. Zookeeper server holds onto dead/expired session ids in the watch data structures
   (Germán Blanco and Michael Morello via camille)
 
+  ZOOKEEPER-1839. Deadlock in NettyServerCnxn (Rakesh R via michim)
+
 IMPROVEMENTS:
 
   ZOOKEEPER-1170. Fix compiler (eclipse) warnings: unused imports,

src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java

@@ -108,12 +108,12 @@ public class NettyServerCnxn extends ServerCnxn {
                             .getRemoteAddress()).getAddress());
                 s.remove(this);
             }
-    
-            if (channel.isOpen()) {
-                channel.close();
-            }
-            factory.unregisterConnection(this);
         }
+
+        if (channel.isOpen()) {
+            channel.close();
+        }
+        factory.unregisterConnection(this);
     }
 
     @Override

src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java

@@ -261,20 +261,22 @@ public class NettyServerCnxnFactory extends ServerCnxnFactory {
             LOG.debug("closeAll()");
         }
 
+        NettyServerCnxn[] allCnxns = null;
         synchronized (cnxns) {
-            // got to clear all the connections that we have in the selector
-            for (NettyServerCnxn cnxn : cnxns.toArray(new NettyServerCnxn[cnxns.size()])) {
-                try {
-                    cnxn.close();
-                } catch (Exception e) {
-                    LOG.warn("Ignoring exception closing cnxn sessionid 0x"
-                            + Long.toHexString(cnxn.getSessionId()), e);
-                }
+            allCnxns = cnxns.toArray(new NettyServerCnxn[cnxns.size()]);
+        }
+        // got to clear all the connections that we have in the selector
+        for (NettyServerCnxn cnxn : allCnxns) {
+            try {
+                cnxn.close();
+            } catch (Exception e) {
+                LOG.warn("Ignoring exception closing cnxn sessionid 0x"
+                                + Long.toHexString(cnxn.getSessionId()), e);
             }
         }
         if (LOG.isDebugEnabled()) {
-            LOG.debug("allChannels size:" + allChannels.size()
-                    + " cnxns size:" + cnxns.size());
+            LOG.debug("allChannels size:" + allChannels.size() + " cnxns size:"
+                    + allCnxns.length);
         }
     }
 
@@ -283,17 +285,18 @@ public class NettyServerCnxnFactory extends ServerCnxnFactory {
         if (LOG.isDebugEnabled()) {
             LOG.debug("closeSession sessionid:0x" + sessionId);
         }
-
+        NettyServerCnxn[] allCnxns = null;
         synchronized (cnxns) {
-            for (NettyServerCnxn cnxn : cnxns.toArray(new NettyServerCnxn[cnxns.size()])) {
-                if (cnxn.getSessionId() == sessionId) {
-                    try {
-                        cnxn.close();
-                    } catch (Exception e) {
-                        LOG.warn("exception during session close", e);
-                    }
-                    break;
+            allCnxns = cnxns.toArray(new NettyServerCnxn[cnxns.size()]);
+        }
+        for (NettyServerCnxn cnxn : allCnxns) {
+            if (cnxn.getSessionId() == sessionId) {
+                try {
+                    cnxn.close();
+                } catch (Exception e) {
+                    LOG.warn("exception during session close", e);
                 }
+                break;
             }
         }
     }