Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/680#discussion_r239138642
--- Diff:
zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java ---
@@ -446,4 +458,119 @@ private void configureSSLServerSocket(SSLServerSocket
sslServerSocket) {
LOG.debug("Using Java8-optimized cipher suites for Java version
{}", javaVersion);
return DEFAULT_CIPHERS_JAVA8;
}
+
+ /**
+ * 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();
+ String keyStoreLocation =
config.getProperty(sslKeystoreLocationProperty);
+ if (keyStoreLocation != null && !keyStoreLocation.isEmpty()) {
+ final Path filePath =
Paths.get(keyStoreLocation).toAbsolutePath();
+ Path parentPath = filePath.getParent();
+ if (parentPath == null) {
+ throw new IOException(
+ "Key store path does not have a parent: " +
filePath);
+ }
+ FileChangeWatcher newKeyStoreFileWatcher = new
FileChangeWatcher(
+ parentPath,
+ watchEvent -> {
+ handleWatchEvent(filePath, watchEvent);
+ });
+ // stop old watcher if there is one
+ if (keyStoreFileWatcher != null) {
+ keyStoreFileWatcher.stop();
+ }
+ keyStoreFileWatcher = newKeyStoreFileWatcher;
+ keyStoreFileWatcher.start();
+ }
+ String trustStoreLocation =
config.getProperty(sslTruststoreLocationProperty);
+ if (trustStoreLocation != null && !trustStoreLocation.isEmpty()) {
+ final Path filePath =
Paths.get(trustStoreLocation).toAbsolutePath();
+ Path parentPath = filePath.getParent();
+ if (parentPath == null) {
+ throw new IOException(
+ "Trust store path does not have a parent: " +
filePath);
+ }
+ FileChangeWatcher newTrustStoreFileWatcher = new
FileChangeWatcher(
+ parentPath,
+ watchEvent -> {
+ handleWatchEvent(filePath, watchEvent);
+ });
+ // 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.
+ */
+ public void disableCertFileReloading() {
+ if (keyStoreFileWatcher != null) {
+ keyStoreFileWatcher.stop();
+ keyStoreFileWatcher = null;
+ }
+ if (trustStoreFileWatcher != null) {
+ trustStoreFileWatcher.stop();
+ trustStoreFileWatcher = null;
+ }
+ }
+
+ // Finalizer guardian object, see Effective Java item 7
+ // TODO: finalize() is deprecated starting with Java 10. This needs to
be
+ // replaced with an explicit shutdown call.
+ @SuppressWarnings("unused")
+ private final Object finalizerGuardian = new Object() {
--- End diff --
Reading the referenced literature about this, I believe it should be better
to avoid using finalizer like this. We might even be better avoid using
finalizers entirely. There's no guarantee when finalizer gets executed, not
even guarantee to be executed at all, it has a huge performance penalty, etc.
We should rather implement an explicit `close()` method with the
`AutoClosable` interface and call it from QuorumPeer's shutdown() method.
---