zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From an...@apache.org
Subject [zookeeper] branch branch-3.5 updated: ZOOKEEPER-3174: Quorum TLS - support reloading trust/key store
Date Wed, 19 Dec 2018 12:49:08 GMT
This is an automated email from the ASF dual-hosted git repository.

andor pushed a commit to branch branch-3.5
in repository https://gitbox.apache.org/repos/asf/zookeeper.git


The following commit(s) were added to refs/heads/branch-3.5 by this push:
     new 1730774  ZOOKEEPER-3174: Quorum TLS - support reloading trust/key store
1730774 is described below

commit 17307740088a0365e341208d0d985a3da624db90
Author: Ilya Maykov <ilyam@fb.com>
AuthorDate: Wed Dec 19 13:49:04 2018 +0100

    ZOOKEEPER-3174: Quorum TLS - support reloading trust/key store
    
    Allow reloading SSL trust stores and key stores from disk when the files on disk change.
    
    ## Added support for reloading key/trust stores when the file on disk changes
    
    - new property sslQuorumReloadCertFiles which controls the behavior for reloading the key and trust store files for QuorumX509Util. Reloading of key and trust store for ClientX509Util is not in this PR but could be added easily
    - this allows a ZK server to keep running on a machine that uses short-lived certs that refresh frequently without having to restart the ZK process.
    
    This is the branch-3.5 version of #680
    
    Author: Ilya Maykov <ilyam@fb.com>
    
    Reviewers: fangmin@apache.org, andor@apache.org
    
    Closes #737 from ivmaykov/ZOOKEEPER-3174-branch3.5 and squashes the following commits:
    
    6cc1d620f [Ilya Maykov] ZOOKEEPER-3219: Fix flaky FileChangeWatcherTest
    df7294491 [Ilya Maykov] ZOOKEEPER-3174: Quorum TLS - support reloading trust/key store
---
 .../apache/zookeeper/ClientCnxnSocketNetty.java    |  41 +--
 .../zookeeper/client/FourLetterWordMain.java       |  15 +-
 .../apache/zookeeper/common/FileChangeWatcher.java | 253 +++++++++++++++++++
 .../java/org/apache/zookeeper/common/X509Util.java | 115 ++++++++-
 .../java/org/apache/zookeeper/common/ZKConfig.java |  13 +-
 .../zookeeper/server/NettyServerCnxnFactory.java   |   3 +
 .../server/auth/X509AuthenticationProvider.java    |  74 +++---
 .../org/apache/zookeeper/server/quorum/Leader.java |  18 +-
 .../zookeeper/server/quorum/QuorumCnxManager.java  |   4 +-
 .../apache/zookeeper/server/quorum/QuorumPeer.java |   1 +
 .../zookeeper/server/quorum/QuorumPeerConfig.java  |  22 +-
 .../zookeeper/server/quorum/QuorumPeerMain.java    |   3 +
 .../zookeeper/common/FileChangeWatcherTest.java    | 277 +++++++++++++++++++++
 .../org/apache/zookeeper/common/X509UtilTest.java  |  36 +--
 .../server/quorum/QuorumPeerConfigTest.java        |  23 +-
 .../zookeeper/server/quorum/QuorumSSLTest.java     |   4 +-
 .../UnifiedServerSocketModeDetectionTest.java      |   1 +
 .../server/quorum/UnifiedServerSocketTest.java     |   1 +
 .../org/apache/zookeeper/test/ClientSSLTest.java   |   4 +-
 .../org/apache/zookeeper/test/SSLAuthTest.java     |   5 +-
 20 files changed, 791 insertions(+), 122 deletions(-)

diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java
index 34c3db3..a688657 100755
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java
@@ -18,10 +18,27 @@
 
 package org.apache.zookeeper;
 
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.net.SocketAddress;
+import java.util.Iterator;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Semaphore;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+
 import org.apache.zookeeper.ClientCnxn.EndOfStreamException;
 import org.apache.zookeeper.ClientCnxn.Packet;
 import org.apache.zookeeper.client.ZKClientConfig;
 import org.apache.zookeeper.common.ClientX509Util;
+import org.apache.zookeeper.common.X509Util;
 import org.jboss.netty.bootstrap.ClientBootstrap;
 import org.jboss.netty.buffer.ChannelBuffer;
 import org.jboss.netty.buffer.ChannelBuffers;
@@ -42,22 +59,6 @@ import org.jboss.netty.handler.ssl.SslHandler;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.net.ssl.SSLContext;
-import javax.net.ssl.SSLEngine;
-
-import java.io.IOException;
-import java.net.InetSocketAddress;
-import java.net.SocketAddress;
-import java.util.Iterator;
-import java.util.List;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.Executors;
-import java.util.concurrent.Semaphore;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
-
 import static org.apache.zookeeper.common.X509Exception.SSLContextException;
 
 /**
@@ -370,9 +371,11 @@ public class ClientCnxnSocketNetty extends ClientCnxnSocket {
         // Basically we only need to create it once.
         private synchronized void initSSL(ChannelPipeline pipeline) throws SSLContextException {
             if (sslContext == null || sslEngine == null) {
-                sslContext = new ClientX509Util().createSSLContext(clientConfig);
-                sslEngine = sslContext.createSSLEngine(host,port);
-                sslEngine.setUseClientMode(true);
+                try (X509Util x509Util = new ClientX509Util()) {
+                    sslContext = x509Util.createSSLContext(clientConfig);
+                    sslEngine = sslContext.createSSLEngine(host, port);
+                    sslEngine.setUseClientMode(true);
+                }
             }
             pipeline.addLast("ssl", new SslHandler(sslEngine));
             LOG.info("SSL handler added for channel: {}", pipeline.getChannel());
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/client/FourLetterWordMain.java b/zookeeper-server/src/main/java/org/apache/zookeeper/client/FourLetterWordMain.java
index 41f5e9d..a98953f 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/client/FourLetterWordMain.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/client/FourLetterWordMain.java
@@ -34,6 +34,7 @@ import javax.net.ssl.SSLSocketFactory;
 import org.apache.zookeeper.common.ClientX509Util;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.zookeeper.common.X509Exception.SSLContextException;
+import org.apache.zookeeper.common.X509Util;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -90,12 +91,14 @@ public class FourLetterWordMain {
             new InetSocketAddress(InetAddress.getByName(null), port);
         if (secure) {
             LOG.info("using secure socket");
-            SSLContext sslContext = new ClientX509Util().getDefaultSSLContext();
-            SSLSocketFactory socketFactory = sslContext.getSocketFactory();
-            SSLSocket sslSock = (SSLSocket) socketFactory.createSocket();
-            sslSock.connect(hostaddress, timeout);
-            sslSock.startHandshake();
-            sock = sslSock;
+            try (X509Util x509Util = new ClientX509Util()) {
+                SSLContext sslContext = x509Util.getDefaultSSLContext();
+                SSLSocketFactory socketFactory = sslContext.getSocketFactory();
+                SSLSocket sslSock = (SSLSocket) socketFactory.createSocket();
+                sslSock.connect(hostaddress, timeout);
+                sslSock.startHandshake();
+                sock = sslSock;
+            }
         } else {
             sock = new Socket();
             sock.connect(hostaddress, timeout);
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java
new file mode 100644
index 0000000..8b49be9
--- /dev/null
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java
@@ -0,0 +1,253 @@
+/**
+ * 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.common;
+
+import com.sun.nio.file.SensitivityWatchEventModifier;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.ClosedWatchServiceException;
+import java.nio.file.FileSystem;
+import java.nio.file.Path;
+import java.nio.file.StandardWatchEventKinds;
+import java.nio.file.WatchEvent;
+import java.nio.file.WatchKey;
+import java.nio.file.WatchService;
+import java.util.function.Consumer;
+
+/**
+ * Instances of this class can be used to watch a directory for file changes. When a file is added to, deleted from,
+ * or is modified in the given directory, the callback provided by the user will be called from a background thread.
+ * Some things to keep in mind:
+ * <ul>
+ * <li>The callback should be thread-safe.</li>
+ * <li>Changes that happen around the time the thread is started may be missed.</li>
+ * <li>There is a delay between a file changing and the callback firing.</li>
+ * <li>The watch is not recursive - changes to subdirectories will not trigger a callback.</li>
+ * </ul>
+ */
+public final class FileChangeWatcher {
+    private static final Logger LOG = LoggerFactory.getLogger(FileChangeWatcher.class);
+
+    public enum State {
+        NEW,      // object created but start() not called yet
+        STARTING, // start() called but background thread has not entered main loop
+        RUNNING,  // background thread is running
+        STOPPING, // stop() called but background thread has not exited main loop
+        STOPPED   // stop() called and background thread has exited, or background thread crashed
+    }
+
+    private final WatcherThread watcherThread;
+    private State state; // protected by synchronized(this)
+
+    /**
+     * Creates a watcher that watches <code>dirPath</code> and invokes <code>callback</code> on changes.
+     *
+     * @param dirPath the directory to watch.
+     * @param callback the callback to invoke with events. <code>event.kind()</code> will return the type of event,
+     *                 and <code>event.context()</code> will return the filename relative to <code>dirPath</code>.
+     * @throws IOException if there is an error creating the WatchService.
+     */
+    public FileChangeWatcher(Path dirPath, Consumer<WatchEvent<?>> callback) throws IOException {
+        FileSystem fs = dirPath.getFileSystem();
+        WatchService watchService = fs.newWatchService();
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("Registering with watch service: " + dirPath);
+        }
+        dirPath.register(
+                watchService,
+                new WatchEvent.Kind<?>[]{
+                        StandardWatchEventKinds.ENTRY_CREATE,
+                        StandardWatchEventKinds.ENTRY_DELETE,
+                        StandardWatchEventKinds.ENTRY_MODIFY,
+                        StandardWatchEventKinds.OVERFLOW},
+                SensitivityWatchEventModifier.HIGH);
+        state = State.NEW;
+        this.watcherThread = new WatcherThread(watchService, callback);
+        this.watcherThread.setDaemon(true);
+    }
+
+    /**
+     * Returns the current {@link FileChangeWatcher.State}.
+     * @return the current state.
+     */
+    public synchronized State getState() {
+        return state;
+    }
+
+    /**
+     * Blocks until the current state becomes <code>desiredState</code>.
+     * Currently only used by tests, thus package-private.
+     * @param desiredState the desired state.
+     * @throws InterruptedException if the current thread gets interrupted.
+     */
+    synchronized void waitForState(State desiredState) throws InterruptedException {
+        while (this.state != desiredState) {
+            this.wait();
+        }
+    }
+
+    /**
+     * Sets the state to <code>newState</code>.
+     * @param newState the new state.
+     */
+    private synchronized void setState(State newState) {
+        state = newState;
+        this.notifyAll();
+    }
+
+    /**
+     * Atomically sets the state to <code>update</code> if and only if the
+     * state is currently <code>expected</code>.
+     * @param expected the expected state.
+     * @param update the new state.
+     * @return true if the update succeeds, or false if the current state
+     *         does not equal <code>expected</code>.
+     */
+    private synchronized boolean compareAndSetState(State expected, State update) {
+        if (state == expected) {
+            setState(update);
+            return true;
+        } else {
+            return false;
+        }
+    }
+
+    /**
+     * Atomically sets the state to <code>update</code> if and only if the
+     * state is currently one of <code>expectedStates</code>.
+     * @param expectedStates the expected states.
+     * @param update the new state.
+     * @return true if the update succeeds, or false if the current state
+     *         does not equal any of the <code>expectedStates</code>.
+     */
+    private synchronized boolean compareAndSetState(State[] expectedStates, State update) {
+        for (State expected : expectedStates) {
+            if (state == expected) {
+                setState(update);
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Tells the background thread to start. Does not wait for it to be running.
+     * Calling this method more than once has no effect.
+     */
+    public void start() {
+        if (!compareAndSetState(State.NEW, State.STARTING)) {
+            // If previous state was not NEW, start() has already been called.
+            return;
+        }
+        this.watcherThread.start();
+    }
+
+    /**
+     * Tells the background thread to stop. Does not wait for it to exit.
+     */
+    public void stop() {
+        if (compareAndSetState(
+                new State[]{State.RUNNING, State.STARTING},
+                State.STOPPING)) {
+            watcherThread.interrupt();
+        }
+    }
+
+    /**
+     * Inner class that implements the watcher thread logic.
+     */
+    private class WatcherThread extends ZooKeeperThread {
+        private static final String THREAD_NAME = "FileChangeWatcher";
+
+        final WatchService watchService;
+        final Consumer<WatchEvent<?>> callback;
+
+        WatcherThread(WatchService watchService, Consumer<WatchEvent<?>> callback) {
+            super(THREAD_NAME);
+            this.watchService = watchService;
+            this.callback = callback;
+        }
+
+        @Override
+        public void run() {
+            try {
+                LOG.info(getName() + " thread started");
+                if (!compareAndSetState(
+                        FileChangeWatcher.State.STARTING,
+                        FileChangeWatcher.State.RUNNING)) {
+                    // stop() called shortly after start(), before
+                    // this thread started running.
+                    FileChangeWatcher.State state = FileChangeWatcher.this.getState();
+                    if (state != FileChangeWatcher.State.STOPPING) {
+                        throw new IllegalStateException("Unexpected state: " + state);
+                    }
+                    return;
+                }
+                runLoop();
+            } catch (Exception e) {
+                LOG.warn("Error in runLoop()", e);
+                throw e;
+            } finally {
+                try {
+                    watchService.close();
+                } catch (IOException e) {
+                    LOG.warn("Error closing watch service", e);
+                }
+                LOG.info(getName() + " thread finished");
+                FileChangeWatcher.this.setState(FileChangeWatcher.State.STOPPED);
+            }
+        }
+
+        private void runLoop() {
+            while (FileChangeWatcher.this.getState() == FileChangeWatcher.State.RUNNING) {
+                WatchKey key;
+                try {
+                    key = watchService.take();
+                } catch (InterruptedException|ClosedWatchServiceException e) {
+                    if (LOG.isDebugEnabled()) {
+                        LOG.debug(getName() + " was interrupted and is shutting down ...");
+                    }
+                    break;
+                }
+                for (WatchEvent<?> event : key.pollEvents()) {
+                    if (LOG.isDebugEnabled()) {
+                        LOG.debug("Got file changed event: " + event.kind() + " with context: " + event.context());
+                    }
+                    try {
+                        callback.accept(event);
+                    } catch (Throwable e) {
+                        LOG.error("Error from callback", e);
+                    }
+                }
+                boolean isKeyValid = key.reset();
+                if (!isKeyValid) {
+                    // This is likely a problem, it means that file reloading is broken, probably because the
+                    // directory we are watching was deleted or otherwise became inaccessible (unmounted, permissions
+                    // changed, ???).
+                    // For now, we log an error and exit the watcher thread.
+                    LOG.error("Watch key no longer valid, maybe the directory is inaccessible?");
+                    break;
+                }
+            }
+        }
+    }
+}
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java
index e3625a5..4ea105b 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java
@@ -19,8 +19,13 @@ package org.apache.zookeeper.common;
 
 
 import java.io.ByteArrayInputStream;
+import java.io.Closeable;
 import java.io.IOException;
 import java.net.Socket;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardWatchEventKinds;
+import java.nio.file.WatchEvent;
 import java.security.GeneralSecurityException;
 import java.security.KeyManagementException;
 import java.security.KeyStore;
@@ -58,7 +63,7 @@ import org.slf4j.LoggerFactory;
  *   Performance testing done by Facebook engineers shows that on Intel x86_64 machines, Java9 performs better with
  *   GCM and Java8 performs better with CBC, so these seem like reasonable defaults.
  */
-public abstract class X509Util {
+public abstract class X509Util implements Closeable, AutoCloseable {
     private static final Logger LOG = LoggerFactory.getLogger(X509Util.class);
 
     static final String DEFAULT_PROTOCOL = "TLSv1.2";
@@ -93,6 +98,8 @@ public abstract class X509Util {
     private String[] cipherSuites;
 
     private AtomicReference<SSLContext> defaultSSLContext = new AtomicReference<>(null);
+    private FileChangeWatcher keyStoreFileWatcher;
+    private FileChangeWatcher trustStoreFileWatcher;
 
     public X509Util() {
         String cipherSuitesInput = System.getProperty(cipherSuitesProperty);
@@ -172,6 +179,11 @@ public abstract class X509Util {
         return result;
     }
 
+    private void resetDefaultSSLContext() throws X509Exception.SSLContextException {
+        SSLContext newContext = createSSLContext();
+        defaultSSLContext.set(newContext);
+    }
+
     private SSLContext createSSLContext() throws SSLContextException {
         /*
          * Since Configuration initializes the key store and trust store related
@@ -446,4 +458,105 @@ public abstract class X509Util {
         LOG.debug("Using Java8-optimized cipher suites for Java version {}", javaVersion);
         return DEFAULT_CIPHERS_JAVA8;
     }
+
+    private FileChangeWatcher newFileChangeWatcher(String fileLocation) throws IOException {
+        if (fileLocation == null || fileLocation.isEmpty()) {
+            return null;
+        }
+        final Path filePath = Paths.get(fileLocation).toAbsolutePath();
+        Path parentPath = filePath.getParent();
+        if (parentPath == null) {
+            throw new IOException(
+                    "Key/trust store path does not have a parent: " + filePath);
+        }
+        return new FileChangeWatcher(
+                parentPath,
+                watchEvent -> {
+                    handleWatchEvent(filePath, watchEvent);
+                });
+    }
+
+    /**
+     * Enables automatic reloading of the trust store and key store files when they change on disk.
+     *
+     * @throws IOException if creating the FileChangeWatcher objects fails.
+     */
+    public void enableCertFileReloading() throws IOException {
+        LOG.info("enabling cert file reloading");
+        ZKConfig config = new ZKConfig();
+        FileChangeWatcher newKeyStoreFileWatcher =
+                newFileChangeWatcher(config.getProperty(sslKeystoreLocationProperty));
+        if (newKeyStoreFileWatcher != null) {
+            // stop old watcher if there is one
+            if (keyStoreFileWatcher != null) {
+                keyStoreFileWatcher.stop();
+            }
+            keyStoreFileWatcher = newKeyStoreFileWatcher;
+            keyStoreFileWatcher.start();
+        }
+        FileChangeWatcher newTrustStoreFileWatcher =
+                newFileChangeWatcher(config.getProperty(sslTruststoreLocationProperty));
+        if (newTrustStoreFileWatcher != null) {
+            // stop old watcher if there is one
+            if (trustStoreFileWatcher != null) {
+                trustStoreFileWatcher.stop();
+            }
+            trustStoreFileWatcher = newTrustStoreFileWatcher;
+            trustStoreFileWatcher.start();
+        }
+    }
+
+    /**
+     * Disables automatic reloading of the trust store and key store files when they change on disk.
+     * Stops background threads and closes WatchService instances.
+     */
+    @Override
+    public void close() {
+        if (keyStoreFileWatcher != null) {
+            keyStoreFileWatcher.stop();
+            keyStoreFileWatcher = null;
+        }
+        if (trustStoreFileWatcher != null) {
+            trustStoreFileWatcher.stop();
+            trustStoreFileWatcher = null;
+        }
+    }
+
+    /**
+     * Handler for watch events that let us know a file we may care about has changed on disk.
+     *
+     * @param filePath the path to the file we are watching for changes.
+     * @param event    the WatchEvent.
+     */
+    private void handleWatchEvent(Path filePath, WatchEvent<?> event) {
+        boolean shouldResetContext = false;
+        Path dirPath = filePath.getParent();
+        if (event.kind().equals(StandardWatchEventKinds.OVERFLOW)) {
+            // If we get notified about possibly missed events, reload the key store / trust store just to be sure.
+            shouldResetContext = true;
+        } else if (event.kind().equals(StandardWatchEventKinds.ENTRY_MODIFY) ||
+                event.kind().equals(StandardWatchEventKinds.ENTRY_CREATE)) {
+            Path eventFilePath = dirPath.resolve((Path) event.context());
+            if (filePath.equals(eventFilePath)) {
+                shouldResetContext = true;
+            }
+        }
+        // Note: we don't care about delete events
+        if (shouldResetContext) {
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Attempting to reset default SSL context after receiving watch event: " +
+                        event.kind() + " with context: " + event.context());
+            }
+            try {
+                this.resetDefaultSSLContext();
+            } catch (SSLContextException e) {
+                throw new RuntimeException(e);
+            }
+        } else {
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Ignoring watch event and keeping previous default SSL context. Event kind: " +
+                        event.kind() + " with context: " + event.context());
+            }
+        }
+    }
 }
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java
index effc0d5..086c07e 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java
@@ -103,12 +103,15 @@ public class ZKConfig {
         properties.put(KINIT_COMMAND, System.getProperty(KINIT_COMMAND));
         properties.put(JGSS_NATIVE, System.getProperty(JGSS_NATIVE));
 
-        ClientX509Util clientX509Util = new ClientX509Util();
-        putSSLProperties(clientX509Util);
-        properties.put(clientX509Util.getSslAuthProviderProperty(),
-                System.getProperty(clientX509Util.getSslAuthProviderProperty()));
+        try (ClientX509Util clientX509Util = new ClientX509Util()) {
+            putSSLProperties(clientX509Util);
+            properties.put(clientX509Util.getSslAuthProviderProperty(),
+                    System.getProperty(clientX509Util.getSslAuthProviderProperty()));
+        }
 
-        putSSLProperties(new QuorumX509Util());
+        try (X509Util x509Util = new QuorumX509Util()) {
+            putSSLProperties(x509Util);
+        }
     }
     
     private void putSSLProperties(X509Util x509Util) {
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java
index 67e0fba..9fc9edc 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java
@@ -461,6 +461,9 @@ public class NettyServerCnxnFactory extends ServerCnxnFactory {
     @Override
     public void shutdown() {
         LOG.info("shutdown called " + localAddress);
+
+        x509Util.close();
+
         if (login != null) {
             login.shutdown();
         }
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java
index d0ca079..22ad070 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java
@@ -68,46 +68,46 @@ public class X509AuthenticationProvider implements AuthenticationProvider {
      */
     public X509AuthenticationProvider() throws X509Exception {
         ZKConfig config = new ZKConfig();
-        X509Util x509Util = new ClientX509Util();
-
-        String keyStoreLocation = config.getProperty(x509Util.getSslKeystoreLocationProperty(), "");
-        String keyStorePassword = config.getProperty(x509Util.getSslKeystorePasswdProperty(), "");
-        String keyStoreTypeProp = config.getProperty(x509Util.getSslKeystoreTypeProperty());
-
-        boolean crlEnabled = Boolean.parseBoolean(config.getProperty(x509Util.getSslCrlEnabledProperty()));
-        boolean ocspEnabled = Boolean.parseBoolean(config.getProperty(x509Util.getSslOcspEnabledProperty()));
-        boolean hostnameVerificationEnabled = Boolean.parseBoolean(
-                config.getProperty(x509Util.getSslHostnameVerificationEnabledProperty()));
-
-        X509KeyManager km = null;
-        X509TrustManager tm = null;
-        if (keyStoreLocation.isEmpty()) {
-            LOG.warn("keystore not specified for client connection");
-        } else {
-            try {
-                km = X509Util.createKeyManager(keyStoreLocation, keyStorePassword, keyStoreTypeProp);
-            } catch (KeyManagerException e) {
-                LOG.error("Failed to create key manager", e);
+        try (X509Util x509Util = new ClientX509Util()) {
+            String keyStoreLocation = config.getProperty(x509Util.getSslKeystoreLocationProperty(), "");
+            String keyStorePassword = config.getProperty(x509Util.getSslKeystorePasswdProperty(), "");
+            String keyStoreTypeProp = config.getProperty(x509Util.getSslKeystoreTypeProperty());
+
+            boolean crlEnabled = Boolean.parseBoolean(config.getProperty(x509Util.getSslCrlEnabledProperty()));
+            boolean ocspEnabled = Boolean.parseBoolean(config.getProperty(x509Util.getSslOcspEnabledProperty()));
+            boolean hostnameVerificationEnabled = Boolean.parseBoolean(
+                    config.getProperty(x509Util.getSslHostnameVerificationEnabledProperty()));
+
+            X509KeyManager km = null;
+            X509TrustManager tm = null;
+            if (keyStoreLocation.isEmpty()) {
+                LOG.warn("keystore not specified for client connection");
+            } else {
+                try {
+                    km = X509Util.createKeyManager(keyStoreLocation, keyStorePassword, keyStoreTypeProp);
+                } catch (KeyManagerException e) {
+                    LOG.error("Failed to create key manager", e);
+                }
             }
-        }
-        
-        String trustStoreLocation = config.getProperty(x509Util.getSslTruststoreLocationProperty(), "");
-        String trustStorePassword = config.getProperty(x509Util.getSslTruststorePasswdProperty(), "");
-        String trustStoreTypeProp = config.getProperty(x509Util.getSslTruststoreTypeProperty());
-
-        if (trustStoreLocation.isEmpty()) {
-            LOG.warn("Truststore not specified for client connection");
-        } else {
-            try {
-                tm = X509Util.createTrustManager(
-                        trustStoreLocation, trustStorePassword, trustStoreTypeProp, crlEnabled, ocspEnabled,
-                        hostnameVerificationEnabled, false);
-            } catch (TrustManagerException e) {
-                LOG.error("Failed to create trust manager", e);
+
+            String trustStoreLocation = config.getProperty(x509Util.getSslTruststoreLocationProperty(), "");
+            String trustStorePassword = config.getProperty(x509Util.getSslTruststorePasswdProperty(), "");
+            String trustStoreTypeProp = config.getProperty(x509Util.getSslTruststoreTypeProperty());
+
+            if (trustStoreLocation.isEmpty()) {
+                LOG.warn("Truststore not specified for client connection");
+            } else {
+                try {
+                    tm = X509Util.createTrustManager(
+                            trustStoreLocation, trustStorePassword, trustStoreTypeProp, crlEnabled, ocspEnabled,
+                            hostnameVerificationEnabled, false);
+                } catch (TrustManagerException e) {
+                    LOG.error("Failed to create trust manager", e);
+                }
             }
+            this.keyManager = km;
+            this.trustManager = tm;
         }
-        this.keyManager = km;
-        this.trustManager = tm;
     }
 
     /**
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java
index 3fce4e8..791737d 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java
@@ -231,21 +231,16 @@ public class Leader {
     
     private final ServerSocket ss;
 
-    Leader(QuorumPeer self,LeaderZooKeeperServer zk) throws IOException, X509Exception {
+    Leader(QuorumPeer self,LeaderZooKeeperServer zk) throws IOException {
         this.self = self;
         this.proposalStats = new BufferStats();
         try {
-            if (self.shouldUsePortUnification()) {
+            if (self.shouldUsePortUnification() || self.isSslQuorum()) {
+                boolean allowInsecureConnection = self.shouldUsePortUnification();
                 if (self.getQuorumListenOnAllIPs()) {
-                    ss = new UnifiedServerSocket(self.getX509Util(), true, self.getQuorumAddress().getPort());
+                    ss = new UnifiedServerSocket(self.getX509Util(), allowInsecureConnection, self.getQuorumAddress().getPort());
                 } else {
-                    ss = new UnifiedServerSocket(self.getX509Util(), true);
-                }
-            } else if (self.isSslQuorum()) {
-                if (self.getQuorumListenOnAllIPs()) {
-                    ss = self.getX509Util().createSSLServerSocket(self.getQuorumAddress().getPort());
-                } else {
-                    ss = self.getX509Util().createSSLServerSocket();
+                    ss = new UnifiedServerSocket(self.getX509Util(), allowInsecureConnection);
                 }
             } else {
                 if (self.getQuorumListenOnAllIPs()) {
@@ -258,9 +253,6 @@ public class Leader {
             if (!self.getQuorumListenOnAllIPs()) {
                 ss.bind(self.getQuorumAddress());
             }
-        } catch (X509Exception e) {
-            LOG.error("Failed to setup ssl server socket", e);
-            throw e;
         } catch (BindException e) {
             if (self.getQuorumListenOnAllIPs()) {
                 LOG.error("Couldn't bind to port " + self.getQuorumAddress().getPort(), e);
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java
index 519b019..153405f 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java
@@ -859,7 +859,7 @@ public class QuorumCnxManager {
                     if (self.shouldUsePortUnification()) {
                         ss = new UnifiedServerSocket(self.getX509Util(), true);
                     } else if (self.isSslQuorum()) {
-                        ss = self.getX509Util().createSSLServerSocket();
+                        ss = new UnifiedServerSocket(self.getX509Util(), false);
                     } else {
                         ss = new ServerSocket();
                     }
@@ -896,7 +896,7 @@ public class QuorumCnxManager {
                         }
                         numRetries = 0;
                     }
-                } catch (IOException|X509Exception e) {
+                } catch (IOException e) {
                     if (shutdown) {
                         break;
                     }
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java
index 4f07951..3b9d33c 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java
@@ -1337,6 +1337,7 @@ public class QuorumPeer extends ZooKeeperThread implements QuorumStats.Provider
     
     public void shutdown() {
         running = false;
+        x509Util.close();
         if (leader != null) {
             leader.shutdown("quorum Peer shutdown");
         }
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
index 80f6243..e0245f7 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
@@ -68,6 +68,7 @@ public class QuorumPeerConfig {
     protected InetSocketAddress secureClientPortAddress;
     protected boolean sslQuorum = false;
     protected boolean shouldUsePortUnification = false;
+    protected boolean sslQuorumReloadCertFiles = false;
     protected File dataDir;
     protected File dataLogDir;
     protected String dynamicConfigFileStr = null;
@@ -309,6 +310,8 @@ public class QuorumPeerConfig {
                 sslQuorum = Boolean.parseBoolean(value);
             } else if (key.equals("portUnification")){
                 shouldUsePortUnification = Boolean.parseBoolean(value);
+            } else if (key.equals("sslQuorumReloadCertFiles")) {
+                sslQuorumReloadCertFiles = Boolean.parseBoolean(value);
             } else if ((key.startsWith("server.") || key.startsWith("group") || key.startsWith("weight")) && zkProp.containsKey("dynamicConfigFile")) {
                 throw new ConfigException("parameter: " + key + " must be in a separate dynamic config file");
             } else if (key.equals(QuorumAuth.QUORUM_SASL_AUTH_ENABLED)) {
@@ -431,15 +434,16 @@ public class QuorumPeerConfig {
      *             provider is not configured.
      */
     private void configureSSLAuth() throws ConfigException {
-        ClientX509Util clientX509Util = new ClientX509Util();
-        String sslAuthProp = "zookeeper.authProvider." + System.getProperty(clientX509Util.getSslAuthProviderProperty(), "x509");
-        if (System.getProperty(sslAuthProp) == null) {
-            if ("zookeeper.authProvider.x509".equals(sslAuthProp)) {
-                System.setProperty("zookeeper.authProvider.x509",
-                        "org.apache.zookeeper.server.auth.X509AuthenticationProvider");
-            } else {
-                throw new ConfigException("No auth provider configured for the SSL authentication scheme '"
-                        + System.getProperty(clientX509Util.getSslAuthProviderProperty()) + "'.");
+        try (ClientX509Util clientX509Util = new ClientX509Util()) {
+            String sslAuthProp = "zookeeper.authProvider." + System.getProperty(clientX509Util.getSslAuthProviderProperty(), "x509");
+            if (System.getProperty(sslAuthProp) == null) {
+                if ("zookeeper.authProvider.x509".equals(sslAuthProp)) {
+                    System.setProperty("zookeeper.authProvider.x509",
+                            "org.apache.zookeeper.server.auth.X509AuthenticationProvider");
+                } else {
+                    throw new ConfigException("No auth provider configured for the SSL authentication scheme '"
+                            + System.getProperty(clientX509Util.getSslAuthProviderProperty()) + "'.");
+                }
             }
         }
     }
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerMain.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerMain.java
index 7c22d81..f4f6b63 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerMain.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerMain.java
@@ -186,6 +186,9 @@ public class QuorumPeerMain {
           quorumPeer.setLearnerType(config.getPeerType());
           quorumPeer.setSyncEnabled(config.getSyncEnabled());
           quorumPeer.setQuorumListenOnAllIPs(config.getQuorumListenOnAllIPs());
+          if (config.sslQuorumReloadCertFiles) {
+              quorumPeer.getX509Util().enableCertFileReloading();
+          }
 
           // sets quorum sasl authentication configurations
           quorumPeer.setQuorumSaslEnabled(config.quorumEnableSasl);
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/FileChangeWatcherTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/common/FileChangeWatcherTest.java
new file mode 100644
index 0000000..2ef6a86
--- /dev/null
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/common/FileChangeWatcherTest.java
@@ -0,0 +1,277 @@
+/**
+ * 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.common;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.StandardWatchEventKinds;
+import java.nio.file.WatchEvent;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.zookeeper.ZKTestCase;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class FileChangeWatcherTest extends ZKTestCase {
+    private static File tempDir;
+    private static File tempFile;
+
+    private static final Logger LOG = LoggerFactory.getLogger(FileChangeWatcherTest.class);
+
+    @BeforeClass
+    public static void createTempFile() throws IOException {
+        tempDir = ClientBase.createEmptyTestDir();
+        tempFile = File.createTempFile("zk_test_", "", tempDir);
+        tempFile.deleteOnExit();
+    }
+
+    @AfterClass
+    public static void cleanupTempDir() {
+        try {
+            FileUtils.deleteDirectory(tempDir);
+        } catch (IOException e) {
+            // ignore
+        }
+    }
+
+    @Test
+    public void testCallbackWorksOnFileChanges() throws IOException, InterruptedException {
+        FileChangeWatcher watcher = null;
+        try {
+            final List<WatchEvent<?>> events = new ArrayList<>();
+            watcher = new FileChangeWatcher(
+                    tempDir.toPath(),
+                    event -> {
+                        LOG.info("Got an update: " + event.kind() + " " + event.context());
+                        // Filter out the extra ENTRY_CREATE events that are
+                        // sometimes seen at the start. Even though we create the watcher
+                        // after the file exists, sometimes we still get a create event.
+                        if (StandardWatchEventKinds.ENTRY_CREATE.equals(event.kind())) {
+                            return;
+                        }
+                        synchronized (events) {
+                            events.add(event);
+                            events.notifyAll();
+                        }
+                    });
+            watcher.start();
+            watcher.waitForState(FileChangeWatcher.State.RUNNING);
+            Thread.sleep(1000L); // XXX hack
+            for (int i = 0; i < 3; i++) {
+                LOG.info("Modifying file, attempt " + (i + 1));
+                FileUtils.writeStringToFile(
+                        tempFile,
+                        "Hello world " + i + "\n",
+                        StandardCharsets.UTF_8,
+                        true);
+                synchronized (events) {
+                    if (events.size() < i + 1) {
+                        events.wait(3000L);
+                    }
+                    assertEquals("Wrong number of events", i + 1, events.size());
+                    WatchEvent<?> event = events.get(i);
+                    assertEquals(StandardWatchEventKinds.ENTRY_MODIFY, event.kind());
+                    assertEquals(tempFile.getName(), event.context().toString());
+                }
+            }
+        } finally {
+            if (watcher != null) {
+                watcher.stop();
+                watcher.waitForState(FileChangeWatcher.State.STOPPED);
+            }
+        }
+    }
+
+    @Test
+    public void testCallbackWorksOnFileTouched() throws IOException, InterruptedException {
+        FileChangeWatcher watcher = null;
+        try {
+            final List<WatchEvent<?>> events = new ArrayList<>();
+            watcher = new FileChangeWatcher(
+                    tempDir.toPath(),
+                    event -> {
+                        LOG.info("Got an update: " + event.kind() + " " + event.context());
+                        // Filter out the extra ENTRY_CREATE events that are
+                        // sometimes seen at the start. Even though we create the watcher
+                        // after the file exists, sometimes we still get a create event.
+                        if (StandardWatchEventKinds.ENTRY_CREATE.equals(event.kind())) {
+                            return;
+                        }
+                        synchronized (events) {
+                            events.add(event);
+                            events.notifyAll();
+                        }
+                    });
+            watcher.start();
+            watcher.waitForState(FileChangeWatcher.State.RUNNING);
+            Thread.sleep(1000L); // XXX hack
+            LOG.info("Touching file");
+            FileUtils.touch(tempFile);
+            synchronized (events) {
+                if (events.isEmpty()) {
+                    events.wait(3000L);
+                }
+                assertFalse(events.isEmpty());
+                WatchEvent<?> event = events.get(0);
+                assertEquals(StandardWatchEventKinds.ENTRY_MODIFY, event.kind());
+                assertEquals(tempFile.getName(), event.context().toString());
+            }
+        } finally {
+            if (watcher != null) {
+                watcher.stop();
+                watcher.waitForState(FileChangeWatcher.State.STOPPED);
+            }
+        }
+    }
+
+    @Test
+    public void testCallbackWorksOnFileAdded() throws IOException, InterruptedException {
+        FileChangeWatcher watcher = null;
+        try {
+            final List<WatchEvent<?>> events = new ArrayList<>();
+            watcher = new FileChangeWatcher(
+                    tempDir.toPath(),
+                    event -> {
+                        LOG.info("Got an update: " + event.kind() + " " + event.context());
+                        synchronized (events) {
+                            events.add(event);
+                            events.notifyAll();
+                        }
+                    });
+            watcher.start();
+            watcher.waitForState(FileChangeWatcher.State.RUNNING);
+            Thread.sleep(1000L); // XXX hack
+            File tempFile2 = File.createTempFile("zk_test_", "", tempDir);
+            tempFile2.deleteOnExit();
+            synchronized (events) {
+                if (events.isEmpty()) {
+                    events.wait(3000L);
+                }
+                assertFalse(events.isEmpty());
+                WatchEvent<?> event = events.get(0);
+                assertEquals(StandardWatchEventKinds.ENTRY_CREATE, event.kind());
+                assertEquals(tempFile2.getName(), event.context().toString());
+            }
+        } finally {
+            if (watcher != null) {
+                watcher.stop();
+                watcher.waitForState(FileChangeWatcher.State.STOPPED);
+            }
+        }
+    }
+
+    @Test
+    public void testCallbackWorksOnFileDeleted() throws IOException, InterruptedException {
+        FileChangeWatcher watcher = null;
+        try {
+            final List<WatchEvent<?>> events = new ArrayList<>();
+            watcher = new FileChangeWatcher(
+                    tempDir.toPath(),
+                    event -> {
+                        LOG.info("Got an update: " + event.kind() + " " + event.context());
+                        // Filter out the extra ENTRY_CREATE events that are
+                        // sometimes seen at the start. Even though we create the watcher
+                        // after the file exists, sometimes we still get a create event.
+                        if (StandardWatchEventKinds.ENTRY_CREATE.equals(event.kind())) {
+                            return;
+                        }
+                        synchronized (events) {
+                            events.add(event);
+                            events.notifyAll();
+                        }
+                    });
+            watcher.start();
+            watcher.waitForState(FileChangeWatcher.State.RUNNING);
+            Thread.sleep(1000L); // XXX hack
+            tempFile.delete();
+            synchronized (events) {
+                if (events.isEmpty()) {
+                    events.wait(3000L);
+                }
+                assertFalse(events.isEmpty());
+                WatchEvent<?> event = events.get(0);
+                assertEquals(StandardWatchEventKinds.ENTRY_DELETE, event.kind());
+                assertEquals(tempFile.getName(), event.context().toString());
+            }
+        } finally {
+            if (watcher != null) {
+                watcher.stop();
+                watcher.waitForState(FileChangeWatcher.State.STOPPED);
+            }
+        }
+    }
+
+    @Test
+    public void testCallbackErrorDoesNotCrashWatcherThread() throws IOException, InterruptedException {
+        FileChangeWatcher watcher = null;
+        try {
+            final AtomicInteger callCount = new AtomicInteger(0);
+            watcher = new FileChangeWatcher(
+                    tempDir.toPath(),
+                    event -> {
+                        LOG.info("Got an update: " + event.kind() + " " + event.context());
+                        int oldValue;
+                        synchronized (callCount) {
+                            oldValue = callCount.getAndIncrement();
+                            callCount.notifyAll();
+                        }
+                        if (oldValue == 0) {
+                            throw new RuntimeException("This error should not crash the watcher thread");
+                        }
+                    });
+            watcher.start();
+            watcher.waitForState(FileChangeWatcher.State.RUNNING);
+            Thread.sleep(1000L); // XXX hack
+            LOG.info("Modifying file");
+            FileUtils.writeStringToFile(tempFile, "Hello world\n", StandardCharsets.UTF_8, true);
+            synchronized (callCount) {
+                while (callCount.get() == 0) {
+                    callCount.wait(3000L);
+                }
+            }
+            LOG.info("Modifying file again");
+            FileUtils.writeStringToFile(tempFile, "Hello world again\n", StandardCharsets.UTF_8, true);
+            synchronized (callCount) {
+                if (callCount.get() == 1) {
+                    callCount.wait(3000L);
+                }
+            }
+            // The value of callCount can exceed 1 only if the callback thread
+            // survives the exception thrown by the first callback.
+            assertTrue(callCount.get() > 1);
+        } finally {
+            if (watcher != null) {
+                watcher.stop();
+                watcher.waitForState(FileChangeWatcher.State.STOPPED);
+            }
+        }
+    }
+}
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java
index 546cf55..1058010 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java
@@ -70,7 +70,9 @@ public class X509UtilTest extends BaseX509ParameterizedTestCase {
 
     @Before
     public void setUp() throws Exception {
-        x509TestContext.setSystemProperties(new ClientX509Util(), KeyStoreFileType.JKS, KeyStoreFileType.JKS);
+        try (X509Util x509util = new ClientX509Util()) {
+            x509TestContext.setSystemProperties(x509util, KeyStoreFileType.JKS, KeyStoreFileType.JKS);
+        }
         System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY, "org.apache.zookeeper.server.NettyServerCnxnFactory");
         System.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET, "org.apache.zookeeper.ClientCnxnSocketNetty");
         x509Util = new ClientX509Util();
@@ -83,12 +85,14 @@ public class X509UtilTest extends BaseX509ParameterizedTestCase {
         System.clearProperty(x509Util.getSslCrlEnabledProperty());
         System.clearProperty(x509Util.getCipherSuitesProperty());
         System.clearProperty(x509Util.getSslProtocolProperty());
+        System.clearProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty());
         System.clearProperty("com.sun.net.ssl.checkRevocation");
         System.clearProperty("com.sun.security.enableCRLDP");
         Security.setProperty("ocsp.enable", Boolean.FALSE.toString());
         Security.setProperty("com.sun.security.enableCRLDP", Boolean.FALSE.toString());
         System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
         System.clearProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET);
+        x509Util.close();
     }
 
     @Test(timeout = 5000)
@@ -358,35 +362,37 @@ public class X509UtilTest extends BaseX509ParameterizedTestCase {
 
     @Test
     public void testGetSslHandshakeDetectionTimeoutMillisProperty() {
-        X509Util x509Util = new ClientX509Util();
         Assert.assertEquals(
                 X509Util.DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS,
                 x509Util.getSslHandshakeTimeoutMillis());
-        try {
-            String newPropertyString = Integer.toString(X509Util.DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS + 1);
-            System.setProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty(), newPropertyString);
-            // Note: need to create a new ClientX509Util to pick up modified property value
+        // Note: need to create a new ClientX509Util each time to pick up modified property value
+        String newPropertyString = Integer.toString(X509Util.DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS + 1);
+        System.setProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty(), newPropertyString);
+        try (X509Util tempX509Util = new ClientX509Util()) {
             Assert.assertEquals(
                     X509Util.DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS + 1,
-                    new ClientX509Util().getSslHandshakeTimeoutMillis());
-            // 0 value not allowed, will return the default
-            System.setProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty(), "0");
+                    tempX509Util.getSslHandshakeTimeoutMillis());
+        }
+        // 0 value not allowed, will return the default
+        System.setProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty(), "0");
+        try (X509Util tempX509Util = new ClientX509Util()) {
             Assert.assertEquals(
                     X509Util.DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS,
-                    new ClientX509Util().getSslHandshakeTimeoutMillis());
-            // Negative value not allowed, will return the default
-            System.setProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty(), "-1");
+                    tempX509Util.getSslHandshakeTimeoutMillis());
+        }
+        // Negative value not allowed, will return the default
+        System.setProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty(), "-1");
+        try (X509Util tempX509Util = new ClientX509Util()) {
             Assert.assertEquals(
                     X509Util.DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS,
-                    new ClientX509Util().getSslHandshakeTimeoutMillis());
-        } finally {
-            System.clearProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty());
+                    tempX509Util.getSslHandshakeTimeoutMillis());
         }
     }
 
     // Warning: this will reset the x509Util
     private void setCustomCipherSuites() {
         System.setProperty(x509Util.getCipherSuitesProperty(), customCipherSuites[0] + "," + customCipherSuites[1]);
+        x509Util.close(); // remember to close old instance before replacing it
         x509Util = new ClientX509Util();
     }
 }
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerConfigTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerConfigTest.java
index 70d8379..5dfd2d0 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerConfigTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerConfigTest.java
@@ -89,17 +89,18 @@ public class QuorumPeerConfigTest {
      * https://issues.apache.org/jira/browse/ZOOKEEPER-2297
      */
     @Test
-    public void testCustomSSLAuth()
-            throws IOException{
-        System.setProperty(new ClientX509Util().getSslAuthProviderProperty(), "y509");
-        QuorumPeerConfig quorumPeerConfig = new QuorumPeerConfig();
-        try {
-            Properties zkProp = getDefaultZKProperties();
-            zkProp.setProperty("secureClientPort", "12345");
-            quorumPeerConfig.parseProperties(zkProp);
-            fail("ConfigException is expected");
-        } catch (ConfigException e) {
-            assertNotNull(e.getMessage());
+    public void testCustomSSLAuth() throws IOException {
+        try (ClientX509Util x509Util = new ClientX509Util()) {
+            System.setProperty(x509Util.getSslAuthProviderProperty(), "y509");
+            QuorumPeerConfig quorumPeerConfig = new QuorumPeerConfig();
+            try {
+                Properties zkProp = getDefaultZKProperties();
+                zkProp.setProperty("secureClientPort", "12345");
+                quorumPeerConfig.parseProperties(zkProp);
+                fail("ConfigException is expected");
+            } catch (ConfigException e) {
+                assertNotNull(e.getMessage());
+            }
         }
     }
 
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java
index 67c15ad..e47b7ef 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java
@@ -124,7 +124,7 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
     private static final char[] PASSWORD = "testpass".toCharArray();
     private static final String HOSTNAME = "localhost";
 
-    private QuorumX509Util quorumX509Util = new QuorumX509Util();
+    private QuorumX509Util quorumX509Util;
 
     private MainThread q1;
     private MainThread q2;
@@ -156,6 +156,7 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
 
     @Before
     public void setup() throws Exception {
+        quorumX509Util = new QuorumX509Util();
         ClientBase.setupTestEnv();
 
         tmpDir = createTmpDir().getAbsolutePath();
@@ -406,6 +407,7 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
         }
 
         Security.removeProvider("BC");
+        quorumX509Util.close();
     }
 
     private void clearSSLSystemProperties() {
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketModeDetectionTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketModeDetectionTest.java
index 61862a4..e9267b9 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketModeDetectionTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketModeDetectionTest.java
@@ -171,6 +171,7 @@ public class UnifiedServerSocketModeDetectionTest extends ZKTestCase {
         forceClose(clientSocket);
         workerPool.shutdown();
         workerPool.awaitTermination(1000, TimeUnit.MILLISECONDS);
+        x509Util.close();
     }
 
     @Test
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
index 5e4e619..ddc05dc 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
@@ -114,6 +114,7 @@ public class UnifiedServerSocketTest extends BaseX509ParameterizedTestCase {
     @After
     public void tearDown() throws Exception {
         x509TestContext.clearSystemProperties(x509Util);
+        x509Util.close();
     }
 
     private static void forceClose(Socket s) {
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientSSLTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientSSLTest.java
index 08ffb4e..b486077 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientSSLTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientSSLTest.java
@@ -37,10 +37,11 @@ import org.junit.Test;
 
 public class ClientSSLTest extends QuorumPeerTestBase {
 
-    private ClientX509Util clientX509Util = new ClientX509Util();
+    private ClientX509Util clientX509Util;
 
     @Before
     public void setup() {
+        clientX509Util = new ClientX509Util();
         String testDataPath = System.getProperty("test.data.dir", "build/test/data");
         System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY, "org.apache.zookeeper.server.NettyServerCnxnFactory");
         System.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET, "org.apache.zookeeper.ClientCnxnSocketNetty");
@@ -60,6 +61,7 @@ public class ClientSSLTest extends QuorumPeerTestBase {
         System.clearProperty(clientX509Util.getSslKeystorePasswdProperty());
         System.clearProperty(clientX509Util.getSslTruststoreLocationProperty());
         System.clearProperty(clientX509Util.getSslTruststorePasswdProperty());
+        clientX509Util.close();
     }
 
     /**
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/SSLAuthTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/SSLAuthTest.java
index 8fd35bc..6d30343 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/SSLAuthTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/SSLAuthTest.java
@@ -24,7 +24,6 @@ import org.apache.zookeeper.PortAssignment;
 import org.apache.zookeeper.TestableZooKeeper;
 import org.apache.zookeeper.client.ZKClientConfig;
 import org.apache.zookeeper.common.ClientX509Util;
-import org.apache.zookeeper.common.X509Util;
 import org.apache.zookeeper.server.ServerCnxnFactory;
 import org.junit.After;
 import org.junit.Assert;
@@ -33,10 +32,11 @@ import org.junit.Test;
 
 public class SSLAuthTest extends ClientBase {
     
-    private ClientX509Util clientX509Util = new ClientX509Util();
+    private ClientX509Util clientX509Util;
     
     @Before
     public void setUp() throws Exception {
+        clientX509Util = new ClientX509Util();
         String testDataPath = System.getProperty("test.data.dir", "build/test/data");
         System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY, "org.apache.zookeeper.server.NettyServerCnxnFactory");
         System.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET, "org.apache.zookeeper.ClientCnxnSocketNetty");
@@ -71,6 +71,7 @@ public class SSLAuthTest extends ClientBase {
         System.clearProperty(clientX509Util.getSslTruststorePasswdProperty());
         System.clearProperty("javax.net.debug");
         System.clearProperty("zookeeper.authProvider.x509");
+        clientX509Util.close();
     }
 
     @Test


Mime
View raw message