Browse Source

ZOOKEEPER-2111 Not isAlive states should be synchronized in ClientCnxn (Hongchao via rakeshr)

git-svn-id: https://svn.apache.org/repos/asf/zookeeper/trunk@1656167 13f79535-47bb-0310-9956-ffa450edef68
Rakesh Radhakrishnan 10 năm trước cách đây
mục cha
commit
ffb584be6c

+ 3 - 0
CHANGES.txt

@@ -25,6 +25,9 @@ BUGFIXES:
   ZOOKEEPER-2072 Netty Server Should Configure Child Channel Pipeline By Specifying 
   ZOOKEEPER-2072 Netty Server Should Configure Child Channel Pipeline By Specifying 
   ChannelPipelineFactory (Hongchao via rakeshr)
   ChannelPipelineFactory (Hongchao via rakeshr)
 
 
+  ZOOKEEPER-2111 Not isAlive states should be synchronized in ClientCnxn 
+  (Hongchao via rakeshr)
+
 IMPROVEMENTS:
 IMPROVEMENTS:
   ZOOKEEPER-1660 Documentation for Dynamic Reconfiguration (Reed Wanderman-Milne via shralex)  
   ZOOKEEPER-1660 Documentation for Dynamic Reconfiguration (Reed Wanderman-Milne via shralex)  
 
 

+ 19 - 11
src/java/main/org/apache/zookeeper/ClientCnxn.java

@@ -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();
             clientCnxnSocket.close();
             if (state.isAlive()) {
             if (state.isAlive()) {
                 eventThread.queueEvent(new WatchedEvent(Event.EventType.None,
                 eventThread.queueEvent(new WatchedEvent(Event.EventType.None,
@@ -1515,15 +1517,21 @@ public class ClientCnxn {
         packet.clientPath = clientPath;
         packet.clientPath = clientPath;
         packet.serverPath = serverPath;
         packet.serverPath = serverPath;
         packet.watchDeregistration = watchDeregistration;
         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();
         sendThread.getClientCnxnSocket().packetAdded();
         return packet;
         return packet;