From 6c18b7225ae0511495b0bca02db9513a1d409882 Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Mon, 29 Mar 2021 11:13:04 +0900 Subject: [PATCH 1/3] HADOOP-17608. Fix NPE in TestKMS --- .../java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java b/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java index 3d59e6f5be7b7..245dda11af995 100644 --- a/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java +++ b/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java @@ -550,7 +550,7 @@ public Void call() throws Exception { threadGroup.enumerate(threads); Thread reloaderThread = null; for (Thread thread : threads) { - if ((thread.getName() != null) + if (thread != null && thread.getName() != null && (thread.getName().contains(SSL_RELOADER_THREAD_NAME))) { reloaderThread = thread; } From e20f7ea4427cab0ae63ac362ba3a22780fcbff6d Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Tue, 30 Mar 2021 12:45:10 +0900 Subject: [PATCH 2/3] Use ThreadUtils#findThreadsByName instead of ThreadGroup#enumarate --- .../hadoop/crypto/key/kms/server/TestKMS.java | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java b/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java index 245dda11af995..ebbc8c80d728f 100644 --- a/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java +++ b/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java @@ -18,6 +18,8 @@ package org.apache.hadoop.crypto.key.kms.server; import java.util.function.Supplier; + +import org.apache.commons.lang3.ThreadUtils; import org.apache.hadoop.thirdparty.com.google.common.cache.LoadingCache; import org.apache.curator.test.TestingServer; import org.apache.hadoop.conf.Configuration; @@ -525,6 +527,7 @@ public void testStartStop(final boolean ssl, final boolean kerberos) if (ssl) { sslFactory = new SSLFactory(SSLFactory.Mode.CLIENT, conf); try { + // the first reloader thread is created here sslFactory.init(); } catch (GeneralSecurityException ex) { throw new IOException(ex); @@ -541,28 +544,23 @@ public Void call() throws Exception { final URI uri = createKMSUri(getKMSUrl()); if (ssl) { + // the second reloader thread is created here KeyProvider testKp = createProvider(uri, conf); - ThreadGroup threadGroup = Thread.currentThread().getThreadGroup(); - while (threadGroup.getParent() != null) { - threadGroup = threadGroup.getParent(); - } - Thread[] threads = new Thread[threadGroup.activeCount()]; - threadGroup.enumerate(threads); - Thread reloaderThread = null; - for (Thread thread : threads) { - if (thread != null && thread.getName() != null - && (thread.getName().contains(SSL_RELOADER_THREAD_NAME))) { - reloaderThread = thread; - } - } - Assert.assertTrue("Reloader is not alive", reloaderThread.isAlive()); - // Explicitly close the provider so we can verify the internal thread - // is shutdown + Collection reloaderThreads = + ThreadUtils.findThreadsByName(SSL_RELOADER_THREAD_NAME); + // now there are two active reloader threads + assertEquals(2, reloaderThreads.size()); + // Explicitly close the provider so we can verify + // the second reloader thread is shutdown testKp.close(); boolean reloaderStillAlive = true; for (int i = 0; i < 10; i++) { - reloaderStillAlive = reloaderThread.isAlive(); - if (!reloaderStillAlive) break; + for (Thread thread : reloaderThreads) { + if (!thread.isAlive()) { + reloaderStillAlive = false; + break; + } + } Thread.sleep(1000); } Assert.assertFalse("Reloader is still alive", reloaderStillAlive); From 650eb205c4baa0dcebda32b2337a11c57a22313e Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Wed, 31 Mar 2021 15:37:26 +0900 Subject: [PATCH 3/3] Verify that the active reloader threads is reduced by 1 --- .../java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java b/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java index ebbc8c80d728f..dbe685b6a3d8b 100644 --- a/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java +++ b/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java @@ -564,6 +564,9 @@ public Void call() throws Exception { Thread.sleep(1000); } Assert.assertFalse("Reloader is still alive", reloaderStillAlive); + reloaderThreads = + ThreadUtils.findThreadsByName(SSL_RELOADER_THREAD_NAME); + assertEquals(1, reloaderThreads.size()); } if (kerberos) {