ZOOKEEPER-1140. server shutdown is not stopping threads. (laxman via mahadev)

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

CHANGES.txt

@@ -295,6 +295,8 @@ BUGFIXES:
   ZOOKEEPER-1117. zookeeper 3.3.3 fails to build with gcc >= 4.6.1 on
   Debian/Ubuntu (James Page via mahadev)
 
+  ZOOKEEPER-1140. server shutdown is not stopping threads. (laxman via mahadev)
+
 IMPROVEMENTS:
   ZOOKEEPER-724. Improve junit test integration - log harness information 
   (phunt via mahadev)

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

@@ -523,17 +523,17 @@ public class LearnerHandler extends Thread {
             LOG.warn("******* GOODBYE " 
                     + (sock != null ? sock.getRemoteSocketAddress() : "<null>")
                     + " ********");
-            // Send the packet of death
-            try {
-                queuedPackets.put(proposalOfDeath);
-            } catch (InterruptedException e) {
-                LOG.warn("Ignoring unexpected exception", e);
-            }
             shutdown();
         }
     }
 
     public void shutdown() {
+        // Send the packet of death
+        try {
+            queuedPackets.put(proposalOfDeath);
+        } catch (InterruptedException e) {
+            LOG.warn("Ignoring unexpected exception", e);
+        }
         try {
             if (sock != null && !sock.isClosed()) {
                 sock.close();

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

@@ -671,33 +671,36 @@ public class QuorumPeer extends Thread implements QuorumStats.Provider {
                             logFactory, this,
                             new ZooKeeperServer.BasicDataTreeBuilder(),
                             this.zkDb);
-                    try {
-                        // Instead of starting roZk immediately, wait some grace
-                        // period before we decide we're partitioned.
-                        //
-                        // Thread is used here because otherwise it would require
-                        // changes in each of election strategy classes which is
-                        // unnecessary code coupling.
-                        new Thread() {
-                            public void run() {
-                                try {
-                                    // lower-bound grace period to 2 secs
-                                    sleep(Math.max(2000, tickTime));
-                                    if (ServerState.LOOKING.equals(getPeerState())) {
-                                        roZk.startup();
-                                        LOG.info("Read-only server started");
-                                    }
-                                } catch (Exception e) {
-                                    LOG.error("FAILED to start ReadOnlyZooKeeperServer", e);
+
+                    // Instead of starting roZk immediately, wait some grace
+                    // period before we decide we're partitioned.
+                    //
+                    // Thread is used here because otherwise it would require
+                    // changes in each of election strategy classes which is
+                    // unnecessary code coupling.
+                    Thread roZkMgr = new Thread() {
+                        public void run() {
+                            try {
+                                // lower-bound grace period to 2 secs
+                                sleep(Math.max(2000, tickTime));
+                                if (ServerState.LOOKING.equals(getPeerState())) {
+                                    roZk.startup();
                                 }
+                            } catch (Exception e) {
+                                LOG.error("FAILED to start ReadOnlyZooKeeperServer", e);
                             }
-                        }.start();
-
+                        }
+                    };
+                    try {
+                        roZkMgr.start();
                         setCurrentVote(makeLEStrategy().lookForLeader());
                     } catch (Exception e) {
                         LOG.warn("Unexpected exception",e);
                         setPeerState(ServerState.LOOKING);
                     } finally {
+                        // If the thread is in the the grace period, interrupt
+                        // to come out of waiting.
+                        roZkMgr.interrupt();
                         roZk.shutdown();
                     }
                     break;

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

@@ -38,6 +38,7 @@ import org.apache.zookeeper.server.persistence.FileTxnSnapLog;
  */
 public class ReadOnlyZooKeeperServer extends QuorumZooKeeperServer {
 
+    private volatile boolean shutdown = false;
     ReadOnlyZooKeeperServer(FileTxnSnapLog logFactory, QuorumPeer self,
             DataTreeBuilder treeBuilder, ZKDatabase zkDb) {
         super(logFactory, self.tickTime, self.minSessionTimeout, self.maxSessionTimeout,
@@ -54,11 +55,16 @@ public class ReadOnlyZooKeeperServer extends QuorumZooKeeperServer {
     }
 
     @Override
-    public void startup() {
+    public synchronized void startup() {
+        // check to avoid startup follows shutdown
+        if (shutdown) {
+            LOG.warn("Not starting Read-only server as startup follows shutdown!");
+            return;
+        }
         registerJMX(new ReadOnlyBean(this), self.jmxLocalPeerBean);
         super.startup();
-
         self.cnxnFactory.setZooKeeperServer(this);
+        LOG.info("Read-only server started");
     }
 
     @Override
@@ -124,7 +130,8 @@ public class ReadOnlyZooKeeperServer extends QuorumZooKeeperServer {
     }
 
     @Override
-    public void shutdown() {
+    public synchronized void shutdown() {
+        shutdown = true;
         unregisterJMX(this);
 
         // set peer's server to null