Mailing-List: contact commits-help@zookeeper.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@<zookeeper.apache.org>
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1521308 - in /zookeeper/trunk: CHANGES.txt
 src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java
 src/java/test/org/apache/zookeeper/server/NIOServerCnxnTest.java
Date: Mon, 09 Sep 2013 22:37:14 -0000
To: commits@zookeeper.apache.org
From: michim@apache.org
Message-Id: <20130909223714.5A7032388994@eris.apache.org>

Author: michim
Date: Mon Sep  9 22:37:13 2013
New Revision: 1521308

URL: http://svn.apache.org/r1521308
Log:
ZOOKEEPER-1750 Race condition producing NPE in NIOServerCnxn.toString
(Rakesh R via michim)

Added:
    zookeeper/trunk/src/java/test/org/apache/zookeeper/server/NIOServerCnxnTest.java
Modified:
    zookeeper/trunk/CHANGES.txt
    zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java

Modified: zookeeper/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/zookeeper/trunk/CHANGES.txt?rev=1521308&r1=1521307&r2=1521308&view=diff
==============================================================================
--- zookeeper/trunk/CHANGES.txt (original)
+++ zookeeper/trunk/CHANGES.txt Mon Sep  9 22:37:13 2013
@@ -557,6 +557,9 @@ IMPROVEMENTS:
 
   ZOOKEEPER-1679. c client: use -Wdeclaration-after-statement (michi via fpj)
 
+  ZOOKEEPER-1750 Race condition producing NPE in NIOServerCnxn.toString
+  (Rakesh R via michim)
+
 Release 3.4.0 - 
 
 Non-backward compatible changes:

Modified: zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java
URL: http://svn.apache.org/viewvc/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java?rev=1521308&r1=1521307&r2=1521308&view=diff
==============================================================================
--- zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java (original)
+++ zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java Mon Sep  9 22:37:13 2013
@@ -31,7 +31,6 @@ import java.nio.channels.SelectionKey;
 import java.nio.channels.SocketChannel;
 import java.util.List;
 import java.util.Queue;
-import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.LinkedBlockingQueue;
@@ -63,7 +62,7 @@ public class NIOServerCnxn extends Serve
 
     private final NIOServerCnxnFactory factory;
 
-    private SocketChannel sock;
+    private final SocketChannel sock;
 
     private final SelectorThread selectorThread;
 
@@ -139,7 +138,7 @@ public class NIOServerCnxn extends Serve
             * a tight while loop
             */
            if (bb != ServerCnxnFactory.closeConn) {
-               if (sock != null) {
+               if (sock.isOpen()) {
                    sock.configureBlocking(true);
                    sock.write(bb);
                }
@@ -307,7 +306,7 @@ public class NIOServerCnxn extends Serve
      */
     void doIO(SelectionKey k) throws InterruptedException {
         try {
-            if (sock == null) {
+            if (sock.isOpen() == false) {
                 LOG.warn("trying to do i/o on a null socket for session:0x"
                          + Long.toHexString(sessionId));
 
@@ -993,7 +992,7 @@ public class NIOServerCnxn extends Serve
      * Close resources associated with the sock of this cnxn.
      */
     private void closeSock() {
-        if (sock == null) {
+        if (sock.isOpen() == false) {
             return;
         }
 
@@ -1003,14 +1002,13 @@ public class NIOServerCnxn extends Serve
                         " which had sessionid 0x" + Long.toHexString(sessionId) :
                         " (no session established for client)"));
         closeSock(sock);
-        sock = null;
     }
 
     /**
      * Close resources associated with a sock.
      */
     public static void closeSock(SocketChannel sock) {
-        if (sock == null) {
+        if (sock.isOpen() == false) {
             return;
         }
 
@@ -1152,14 +1150,14 @@ public class NIOServerCnxn extends Serve
 
     @Override
     public InetSocketAddress getRemoteSocketAddress() {
-        if (sock == null) {
+        if (sock.isOpen() == false) {
             return null;
         }
         return (InetSocketAddress) sock.socket().getRemoteSocketAddress();
     }
 
     public InetAddress getSocketAddress() {
-        if (sock == null) {
+        if (sock.isOpen() == false) {
             return null;
         }
         return sock.socket().getInetAddress();

Added: zookeeper/trunk/src/java/test/org/apache/zookeeper/server/NIOServerCnxnTest.java
URL: http://svn.apache.org/viewvc/zookeeper/trunk/src/java/test/org/apache/zookeeper/server/NIOServerCnxnTest.java?rev=1521308&view=auto
==============================================================================
--- zookeeper/trunk/src/java/test/org/apache/zookeeper/server/NIOServerCnxnTest.java (added)
+++ zookeeper/trunk/src/java/test/org/apache/zookeeper/server/NIOServerCnxnTest.java Mon Sep  9 22:37:13 2013
@@ -0,0 +1,64 @@
+/**
+ * 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.server;
+
+import java.io.IOException;
+
+import junit.framework.Assert;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.Test;
+
+public class NIOServerCnxnTest extends ClientBase {
+
+    /**
+     * Test operations on ServerCnxn after socket closure.
+     */
+    @Test(timeout = 60000)
+    public void testOperationsAfterCnxnClose() throws IOException,
+            InterruptedException, KeeperException {
+        final ZooKeeper zk = createClient();
+
+        final String path = "/a";
+        try {
+            // make sure zkclient works
+            zk.create(path, "test".getBytes(), Ids.OPEN_ACL_UNSAFE,
+                    CreateMode.PERSISTENT);
+            Assert.assertNotNull("Didn't create znode:" + path,
+                    zk.exists(path, false));
+            // Defaults ServerCnxnFactory would be instantiated with
+            // NIOServerCnxnFactory
+            Assert.assertTrue(
+                    "Didn't instantiate ServerCnxnFactory with NIOServerCnxnFactory!",
+                    serverFactory instanceof NIOServerCnxnFactory);
+            Iterable<ServerCnxn> connections = serverFactory.getConnections();
+            for (ServerCnxn serverCnxn : connections) {
+                String cnxnStr = serverCnxn.toString();
+                serverCnxn.close();
+                Assert.assertEquals("Connection mismatches!", cnxnStr,
+                        serverCnxn.toString());
+            }
+        } finally {
+            zk.close();
+        }
+    }
+}