ZOOKEEPER-1620. NIOServerCnxnFactory (new code introduced in ZK-1504) opens selectors but never closes them (Thawan Kooburat via phunt)

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

CHANGES.txt

@@ -307,6 +307,10 @@ BUGFIXES:
   ZOOKEEPER-1495. ZK client hangs when using a function not available
   on the server. (Skye W-M via phunt)
 
+  ZOOKEEPER-1620. NIOServerCnxnFactory (new code introduced in
+  ZK-1504) opens selectors but never closes them
+  (Thawan Kooburat via phunt)
+
 IMPROVEMENTS:
 
   ZOOKEEPER-1170. Fix compiler (eclipse) warnings: unused imports,

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

@@ -132,6 +132,20 @@ public class NIOServerCnxnFactory extends ServerCnxnFactory {
             selector.wakeup();
         }
 
+        /**
+         * Close the selector. This should be called when the thread is about to
+         * exit and no operation is going to be performed on the Selector or
+         * SelectionKey
+         */
+        protected void closeSelector() {
+            try {
+                selector.close();
+            } catch (IOException e) {
+                LOG.warn("ignored exception during selector close "
+                        + e.getMessage());
+            }
+        }
+
         protected void cleanupSelectionKey(SelectionKey key) {
             if (key != null) {
                 try {
@@ -195,6 +209,7 @@ public class NIOServerCnxnFactory extends ServerCnxnFactory {
                     }
                 }
             } finally {
+                closeSelector();
                 // This will wake up the selector threads, and tell the
                 // worker thread pool to begin shutdown.
                 NIOServerCnxnFactory.this.stop();
@@ -391,6 +406,7 @@ public class NIOServerCnxnFactory extends ServerCnxnFactory {
                 }
                 updateQueue.clear();
             } finally {
+                closeSelector();
                 // This will wake up the accept thread and the other selector
                 // threads, and tell the worker thread pool to begin shutdown.
                 NIOServerCnxnFactory.this.stop();

src/java/test/org/apache/zookeeper/test/NIOConnectionFactoryFdLeakTest.java

@@ -0,0 +1,68 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+import java.net.InetSocketAddress;
+
+import org.apache.zookeeper.PortAssignment;
+import org.apache.zookeeper.ZKTestCase;
+import org.apache.zookeeper.server.NIOServerCnxnFactory;
+import org.apache.zookeeper.server.util.OSMXBean;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * ZOOKEEPER-1620 - Acceptor and Selector thread don't call selector.close()
+ * causing fd leakage
+ */
+public class NIOConnectionFactoryFdLeakTest extends ZKTestCase {
+    private static final Logger LOG = LoggerFactory
+            .getLogger(NIOConnectionFactoryFdLeakTest.class);
+
+    @Test
+    public void testFileDescriptorLeak() throws Exception {
+
+        OSMXBean osMbean = new OSMXBean();
+        if (osMbean.getUnix() != true) {
+            LOG.info("Unable to run test on non-unix system");
+            return;
+        }
+
+        long startFdCount = osMbean.getOpenFileDescriptorCount();
+        LOG.info("Start fdcount is: " + startFdCount);
+
+        for (int i = 0; i < 50; ++i) {
+            NIOServerCnxnFactory factory = new NIOServerCnxnFactory();
+            factory.configure(new InetSocketAddress(PortAssignment.unique()),
+                    10);
+            factory.start();
+            Thread.sleep(100);
+            factory.shutdown();
+        }
+
+        long endFdCount = osMbean.getOpenFileDescriptorCount();
+        LOG.info("End fdcount is: " + endFdCount);
+
+        // On my box, if selector.close() is not called fd diff is > 700.
+        Assert.assertTrue("Possible fd leakage",
+                ((endFdCount - startFdCount) < 50));
+    }
+}