ZOOKEEPER-1208. Ephemeral node not removed after the client session is long gone. (phunt via camille)

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

CHANGES.txt

@@ -54,6 +54,8 @@ BUGFIXES:
   
   ZOOKEEPER-1291. AcceptedEpoch not updated at leader before it proposes the epoch to followers. (Alex Shraer via camille)
 
+  ZOOKEEPER-1208. Ephemeral node not removed after the client session is long gone. (phunt via camille)
+  
 IMPROVEMENTS:
 
   ZOOKEEPER-1170. Fix compiler (eclipse) warnings: unused imports,

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

@@ -438,7 +438,10 @@ public class PrepRequestProcessor extends Thread implements RequestProcessor {
                     for (String path2Delete : es) {
                         addChangeRecord(new ChangeRecord(request.getHdr().getZxid(), path2Delete, null, 0, null));
                     }
+
+                    zks.sessionTracker.setSessionClosing(request.sessionId);
                 }
+
                 LOG.info("Processed session termination for sessionid: 0x"
                         + Long.toHexString(request.sessionId));
                 break;

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

@@ -34,6 +34,7 @@ public interface SessionTracker {
     public static interface Session {
         long getSessionId();
         int getTimeout();
+        boolean isClosing();
     }
     public static interface SessionExpirer {
         void expire(Session session);
@@ -52,6 +53,12 @@ public interface SessionTracker {
      */
     boolean touchSession(long sessionId, int sessionTimeout);
 
+    /**
+     * Mark that the session is in the process of closing.
+     * @param sessionId
+     */
+    void setSessionClosing(long sessionId);
+
     /**
      * 
      */

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

@@ -57,16 +57,19 @@ public class SessionTrackerImpl extends Thread implements SessionTracker {
             this.sessionId = sessionId;
             this.timeout = timeout;
             this.tickTime = expireTime;
+            isClosing = false;
         }
 
         final long sessionId;
         final int timeout;
         long tickTime;
+        boolean isClosing;
 
         Object owner;
 
         public long getSessionId() { return sessionId; }
         public int getTimeout() { return timeout; }
+        public boolean isClosing() { return isClosing; }
     }
 
     public static long initializeNextSession(long id) {
@@ -189,6 +192,17 @@ public class SessionTrackerImpl extends Thread implements SessionTracker {
         return true;
     }
 
+    synchronized public void setSessionClosing(long sessionId) {
+        if (LOG.isTraceEnabled()) {
+            LOG.info("Session closing: 0x" + Long.toHexString(sessionId));
+        }
+        SessionImpl s = sessionsById.get(sessionId);
+        if (s == null) {
+            return;
+        }
+        s.isClosing = true;
+    }
+
     synchronized public void removeSession(long sessionId) {
         SessionImpl s = sessionsById.remove(sessionId);
         sessionsWithTimeout.remove(sessionId);
@@ -240,7 +254,7 @@ public class SessionTrackerImpl extends Thread implements SessionTracker {
 
     synchronized public void checkSession(long sessionId, Object owner) throws KeeperException.SessionExpiredException, KeeperException.SessionMovedException {
         SessionImpl session = sessionsById.get(sessionId);
-        if (session == null) {
+        if (session == null || session.isClosing()) {
             throw new KeeperException.SessionExpiredException();
         }
         if (session.owner == null) {

src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java

@@ -89,4 +89,8 @@ public class LearnerSessionTracker implements SessionTracker {
     	// dup what we had before
     	pwriter.println(toString());
     }
+
+    public void setSessionClosing(long sessionId) {
+        // Nothing to do here.
+    }
 }

src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java

@@ -117,5 +117,9 @@ public class PrepRequestProcessorTest extends ClientBase {
             // TODO Auto-generated method stub
             return false;
         }
+        @Override
+        public void setSessionClosing(long sessionId) {
+          // TODO Auto-generated method stub
+        }
     }
 }

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

@@ -0,0 +1,105 @@
+/**
+ * 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.io.ByteArrayOutputStream;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.net.Socket;
+
+import junit.framework.Assert;
+
+import org.apache.jute.BinaryOutputArchive;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooDefs.OpCode;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.proto.ConnectRequest;
+import org.apache.zookeeper.proto.CreateRequest;
+import org.apache.zookeeper.proto.RequestHeader;
+import org.junit.Test;
+
+public class SessionInvalidationTest extends ClientBase {
+    /**
+     * Test solution for ZOOKEEPER-1208. Verify that operations are not
+     * accepted after a close session.
+     * 
+     * We're using our own marshalling here in order to force an operation
+     * after the session is closed (ZooKeeper.class will not allow this). Also
+     * by filling the pipe with operations it increases the likelyhood that
+     * the server will process the create before FinalRequestProcessor
+     * removes the session from the tracker.
+     */
+    @Test
+    public void testCreateAfterCloseShouldFail() throws Exception {
+        for (int i = 0; i < 10; i++) {
+            ByteArrayOutputStream baos = new ByteArrayOutputStream();
+            BinaryOutputArchive boa = BinaryOutputArchive.getArchive(baos);
+
+            // open a connection
+            boa.writeInt(44, "len");
+            ConnectRequest conReq = new ConnectRequest(0, 0, 30000, 0, new byte[16]);
+            conReq.serialize(boa, "connect");
+
+            // close connection
+            boa.writeInt(8, "len");
+            RequestHeader h = new RequestHeader(1, ZooDefs.OpCode.closeSession);
+            h.serialize(boa, "header");
+
+            // create ephemeral znode
+            boa.writeInt(52, "len"); // We'll fill this in later
+            RequestHeader header = new RequestHeader(2, OpCode.create);
+            header.serialize(boa, "header");
+            CreateRequest createReq = new CreateRequest("/foo" + i, new byte[0],
+                    Ids.OPEN_ACL_UNSAFE, 1);
+            createReq.serialize(boa, "request");
+            baos.close();
+            
+            System.out.println("Length:" + baos.toByteArray().length);
+            
+            String hp[] = hostPort.split(":");
+            Socket sock = new Socket(hp[0], Integer.parseInt(hp[1]));
+            InputStream resultStream = null;
+            try {
+                OutputStream outstream = sock.getOutputStream();
+                byte[] data = baos.toByteArray();
+                outstream.write(data);
+                outstream.flush();
+                
+                resultStream = sock.getInputStream();
+                byte[] b = new byte[10000];
+                int len;
+                while ((len = resultStream.read(b)) >= 0) {
+                    // got results
+                    System.out.println("gotlen:" + len);
+                }
+            } finally {
+                if (resultStream != null) {
+                    resultStream.close();
+                }
+                sock.close();
+            }
+        }
+        
+        ZooKeeper zk = createClient();
+        Assert.assertEquals(1, zk.getChildren("/", false).size());
+
+        zk.close();
+    }
+}