diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java index 97c07f4e1005..ade505d3f852 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java @@ -53,8 +53,6 @@ import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; import com.google.common.annotations.VisibleForTesting; @@ -122,20 +120,11 @@ public abstract class DefaultCertificateClient implements CertificateClient { private KeyStoresFactory serverKeyStoresFactory; private KeyStoresFactory clientKeyStoresFactory; - // Lock to protect the certificate renew process, to make sure there is only - // one renew process is ongoing at one time. - // Certificate renew steps: - // 1. generate new keys and sign new certificate, persist all data to disk - // 2. switch on disk new keys and certificate with current ones - // 3. save new certificate ID into service VERSION file - // 4. refresh in memory certificate ID and reload all new certificates - private Lock renewLock = new ReentrantLock(); - private ScheduledExecutorService executorService; private Consumer certIdSaveCallback; private Runnable shutdownCallback; private SCMSecurityProtocolClientSideTranslatorPB scmSecurityProtocolClient; - private Set notificationReceivers; + private final Set notificationReceivers; private static UserGroupInformation ugi; protected DefaultCertificateClient(SecurityConfig securityConfig, Logger log, @@ -935,7 +924,8 @@ public void registerNotificationReceiver(CertificateNotification receiver) { @Override public synchronized void close() throws IOException { if (executorService != null) { - executorService.shutdown(); + executorService.shutdownNow(); + executorService = null; } if (serverKeyStoresFactory != null) { @@ -1249,9 +1239,14 @@ public CertificateLifetimeMonitor(CertificateClient client) { @Override public void run() { - - renewLock.lock(); - try { + // Lock to protect the certificate renew process, to make sure there is + // only one renew process is ongoing at one time. + // Certificate renew steps: + // 1. generate new keys and sign new certificate, persist data to disk + // 2. switch on disk new keys and certificate with current ones + // 3. save new certificate ID into service VERSION file + // 4. refresh in memory certificate ID and reload all new certificates + synchronized (DefaultCertificateClient.class) { X509Certificate currentCert = getCertificate(); Duration timeLeft = timeBeforeExpiryGracePeriod(currentCert); if (timeLeft.isZero()) { @@ -1288,8 +1283,6 @@ public void run() { notificationReceivers.forEach(r -> r.notifyCertificateRenewed( certClient, currentCert.getSerialNumber().toString(), newCertId)); } - } finally { - renewLock.unlock(); } } }