svn commit: r1656167 - in /zookeeper/trunk: CHANGES.txt src/java/main/org/apache/zookeeper/ClientCnxn.java

[ra...@apache.org]

Author: rakeshr
Date: Sat Jan 31 07:05:54 2015
New Revision: 1656167

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

Modified:
    zookeeper/trunk/CHANGES.txt
    zookeeper/trunk/src/java/main/org/apache/zookeeper/ClientCnxn.java

Modified: zookeeper/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/zookeeper/trunk/CHANGES.txt?rev=1656167&r1=1656166&r2=1656167&view=diff
==============================================================================
--- zookeeper/trunk/CHANGES.txt (original)
+++ zookeeper/trunk/CHANGES.txt Sat Jan 31 07:05:54 2015
@@ -25,6 +25,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/trunk/src/java/main/org/apache/zookeeper/ClientCnxn.java
URL: http://svn.apache.org/viewvc/zookeeper/trunk/src/java/main/org/apache/zookeeper/ClientCnxn.java?rev=1656167&r1=1656166&r2=1656167&view=diff
==============================================================================
--- zookeeper/trunk/src/java/main/org/apache/zookeeper/ClientCnxn.java (original)
+++ zookeeper/trunk/src/java/main/org/apache/zookeeper/ClientCnxn.java Sat Jan 31 07:05:54 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;