From 5a201d633fd90a14ba76795683be36b05baedff0 Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Tue, 25 Aug 2026 02:50:59 -0400 Subject: [PATCH 1/3] ZOOKEEPER-4947: Close clients after authentication failure ZooKeeper.close() treated AUTH_FAILED clients as already closed and skipped prompt shutdown of their connection threads. Signed-off-by: 1fanwang <1fannnw@gmail.com> --- .../src/main/java/org/apache/zookeeper/ZooKeeper.java | 2 +- .../org/apache/zookeeper/test/SaslAuthFailTest.java | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java b/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java index 239f97b0f70..20e8e2cddd2 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java @@ -1319,7 +1319,7 @@ public synchronized void register(Watcher watcher) { * @throws InterruptedException */ public synchronized void close() throws InterruptedException { - if (!cnxn.getState().isAlive()) { + if (cnxn.getState() == States.CLOSED) { LOG.debug("Close called on already closed client"); return; } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthFailTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthFailTest.java index 2384cd612ef..3f586c247d3 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthFailTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthFailTest.java @@ -18,11 +18,14 @@ package org.apache.zookeeper.test; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; import java.io.File; import java.io.FileWriter; import java.io.IOException; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import org.apache.zookeeper.CreateMode; import org.apache.zookeeper.WatchedEvent; import org.apache.zookeeper.Watcher.Event.KeeperState; @@ -88,9 +91,12 @@ public void testAuthFail() { @Test public void testBadSaslAuthNotifiesWatch() throws Exception { - try (ZooKeeper ignored = createClient(new MyWatcher(), hostPort)) { + try (ZooKeeper zk = createClient(new MyWatcher(), hostPort)) { // wait for authFailed event from client's EventThread. - authFailed.await(); + assertTrue(authFailed.await(CONNECTION_TIMEOUT, TimeUnit.MILLISECONDS)); + zk.close(); + assertEquals(ZooKeeper.States.CLOSED, zk.getState()); + assertTrue(zk.close(CONNECTION_TIMEOUT)); } } From 2dd3c4446b90b701460b5e7efe1c7af5cbc741f8 Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Sat, 19 Sep 2026 23:25:41 -0700 Subject: [PATCH 2/3] ZOOKEEPER-4947: Stop send loop after SASL authentication failure Signed-off-by: 1fanwang <1fannnw@gmail.com> --- .../src/main/java/org/apache/zookeeper/ClientCnxn.java | 1 + .../test/java/org/apache/zookeeper/TestableZooKeeper.java | 5 +++++ .../java/org/apache/zookeeper/test/SaslAuthFailTest.java | 8 +++++++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java index 020f9408aab..e167dff1d1b 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java @@ -1217,6 +1217,7 @@ public void run() { eventThread.queueEvent(new WatchedEvent(Watcher.Event.EventType.None, authState, null)); if (state == States.AUTH_FAILED) { eventThread.queueEventOfDeath(); + break; } } } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/TestableZooKeeper.java b/zookeeper-server/src/test/java/org/apache/zookeeper/TestableZooKeeper.java index 7f9e41e3380..da9e5a7cc2e 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/TestableZooKeeper.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/TestableZooKeeper.java @@ -85,6 +85,11 @@ public void run() { } } + @Override + public boolean testableWaitForShutdown(int wait) throws InterruptedException { + return super.testableWaitForShutdown(wait); + } + public SocketAddress testableLocalSocketAddress() { return super.testableLocalSocketAddress(); } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthFailTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthFailTest.java index 3f586c247d3..c8b419f0cc7 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthFailTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthFailTest.java @@ -19,6 +19,7 @@ package org.apache.zookeeper.test; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; import java.io.File; @@ -27,6 +28,8 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import org.apache.zookeeper.CreateMode; +import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.TestableZooKeeper; import org.apache.zookeeper.WatchedEvent; import org.apache.zookeeper.Watcher.Event.KeeperState; import org.apache.zookeeper.ZooDefs.Ids; @@ -91,9 +94,12 @@ public void testAuthFail() { @Test public void testBadSaslAuthNotifiesWatch() throws Exception { - try (ZooKeeper zk = createClient(new MyWatcher(), hostPort)) { + try (TestableZooKeeper zk = createClient(new MyWatcher(), hostPort)) { // wait for authFailed event from client's EventThread. assertTrue(authFailed.await(CONNECTION_TIMEOUT, TimeUnit.MILLISECONDS)); + assertTrue(zk.testableWaitForShutdown(1000), "Client threads should stop after SASL authentication fails"); + assertEquals(ZooKeeper.States.AUTH_FAILED, zk.getState()); + assertThrows(KeeperException.AuthFailedException.class, () -> zk.exists("/", false)); zk.close(); assertEquals(ZooKeeper.States.CLOSED, zk.getState()); assertTrue(zk.close(CONNECTION_TIMEOUT)); From 888306580053b0cbb1f3690688b26e0bbfc5fcca Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Sun, 20 Sep 2026 00:01:41 -0700 Subject: [PATCH 3/3] ZOOKEEPER-4947: Log the no-close SASL shutdown result Signed-off-by: 1fanwang <1fannnw@gmail.com> --- .../java/org/apache/zookeeper/test/SaslAuthFailTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthFailTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthFailTest.java index c8b419f0cc7..44d936fe054 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthFailTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthFailTest.java @@ -97,7 +97,10 @@ public void testBadSaslAuthNotifiesWatch() throws Exception { try (TestableZooKeeper zk = createClient(new MyWatcher(), hostPort)) { // wait for authFailed event from client's EventThread. assertTrue(authFailed.await(CONNECTION_TIMEOUT, TimeUnit.MILLISECONDS)); - assertTrue(zk.testableWaitForShutdown(1000), "Client threads should stop after SASL authentication fails"); + boolean threadsStopped = zk.testableWaitForShutdown(1000); + LOG.info("SASL failure shutdown without close: threadsStopped={}, state={}", + threadsStopped, zk.getState()); + assertTrue(threadsStopped, "Client threads should stop after SASL authentication fails"); assertEquals(ZooKeeper.States.AUTH_FAILED, zk.getState()); assertThrows(KeeperException.AuthFailedException.class, () -> zk.exists("/", false)); zk.close();