svn commit: r1656168 - in /zookeeper/branches/branch-3.5: CHANGES.txt src/java/main/org/apache/zookeeper/ClientCnxn.java

[ra...@apache.org]

Author: rakeshr
Date: Sat Jan 31 07:08:20 2015
New Revision: 1656168

URL: http://svn.apache.org/r1656168
Log:
ZOOKEEPER-2111 Not isAlive states should be synchronized in ClientCnxn (Hongchao via rakeshr)

Modified:
    zookeeper/branches/branch-3.5/CHANGES.txt
    zookeeper/branches/branch-3.5/src/java/main/org/apache/zookeeper/ClientCnxn.java

Modified: zookeeper/branches/branch-3.5/CHANGES.txt
URL: http://svn.apache.org/viewvc/zookeeper/branches/branch-3.5/CHANGES.txt?rev=1656168&r1=1656167&r2=1656168&view=diff
==============================================================================
--- zookeeper/branches/branch-3.5/CHANGES.txt (original)
+++ zookeeper/branches/branch-3.5/CHANGES.txt Sat Jan 31 07:08:20 2015
@@ -21,6 +21,9 @@ BUGFIXES:
   ZOOKEEPER-2072 Netty Server Should Configure Child Channel Pipeline By Specifying 
   ChannelPipelineFactory (Hongchao via rakeshr)
 
+  ZOOKEEPER-2111 Not isAlive states should be synchronized in ClientCnxn 
+  (Hongchao via rakeshr)
+
 IMPROVEMENTS:
   ZOOKEEPER-1660 Documentation for Dynamic Reconfiguration (Reed Wanderman-Milne via shralex)
 

Modified: zookeeper/branches/branch-3.5/src/java/main/org/apache/zookeeper/ClientCnxn.java
URL: http://svn.apache.org/viewvc/zookeeper/branches/branch-3.5/src/java/main/org/apache/zookeeper/ClientCnxn.java?rev=1656168&r1=1656167&r2=1656168&view=diff
==============================================================================
--- zookeeper/branches/branch-3.5/src/java/main/org/apache/zookeeper/ClientCnxn.java (original)
+++ zookeeper/branches/branch-3.5/src/java/main/org/apache/zookeeper/ClientCnxn.java Sat Jan 31 07:08:20 2015
@@ -1233,9 +1233,11 @@ public class ClientCnxn {
                     }
                 }
             }
-            // When it comes to this point, it guarantees that later queued packet to outgoingQueue will be
-            // notified of death.
-            cleanup();
+            synchronized (state) {
+                // When it comes to this point, it guarantees that later queued
+                // packet to outgoingQueue will be notified of death.
+                cleanup();
+            }
             clientCnxnSocket.close();
             if (state.isAlive()) {
                 eventThread.queueEvent(new WatchedEvent(Event.EventType.None,
@@ -1515,15 +1517,21 @@ public class ClientCnxn {
         packet.clientPath = clientPath;
         packet.serverPath = serverPath;
         packet.watchDeregistration = watchDeregistration;
-        if (!state.isAlive() || closing) {
-            conLossPacket(packet);
-        } else {
-            // If the client is asking to close the session then
-            // mark as closing
-            if (h.getType() == OpCode.closeSession) {
-                closing = true;
+        // The synchronized block here is for two purpose:
+        // 1. synchronize with the final cleanup() in SendThread.run() to avoid race
+        // 2. synchronized against each packet. So if a closeSession packet is added,
+        // later packet will be notified.
+        synchronized (state) {
+            if (!state.isAlive() || closing) {
+                conLossPacket(packet);
+            } else {
+                // If the client is asking to close the session then
+                // mark as closing
+                if (h.getType() == OpCode.closeSession) {
+                    closing = true;
+                }
+                outgoingQueue.add(packet);
             }
-            outgoingQueue.add(packet);
         }
         sendThread.getClientCnxnSocket().packetAdded();
         return packet;