You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by ti...@apache.org on 2023/06/17 13:49:48 UTC
[curator] branch master updated: CURATOR-41: Suppress secondary exception in LockInternals.attemptLock (#466)
This is an automated email from the ASF dual-hosted git repository.
tison pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/curator.git
The following commit(s) were added to refs/heads/master by this push:
new a4f1ef22 CURATOR-41: Suppress secondary exception in LockInternals.attemptLock (#466)
a4f1ef22 is described below
commit a4f1ef22fa4395d3188341f04025e723a84c961b
Author: Kezhu Wang <ke...@apache.org>
AuthorDate: Sat Jun 17 21:49:42 2023 +0800
CURATOR-41: Suppress secondary exception in LockInternals.attemptLock (#466)
Suppress exception from `deleteOurPath` if we are throwing.
---
.../framework/recipes/locks/LockInternals.java | 19 +++++++----
...inessWithFaults.java => TestLockInternals.java} | 38 ++++++++++++++++++++--
.../org/apache/curator/test/BaseClassForTests.java | 2 +-
3 files changed, 49 insertions(+), 10 deletions(-)
diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
index a5f30c40..1be6bd13 100644
--- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
+++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
@@ -222,7 +222,6 @@ public class LockInternals {
private boolean internalLockLoop(long startMillis, Long millisToWait, String ourPath) throws Exception {
boolean haveTheLock = false;
- boolean doDelete = false;
try {
if (revocable.get() != null) {
client.getData().usingWatcher(revocableWatcher).forPath(ourPath);
@@ -247,7 +246,6 @@ public class LockInternals {
millisToWait -= (System.currentTimeMillis() - startMillis);
startMillis = System.currentTimeMillis();
if (millisToWait <= 0) {
- doDelete = true; // timed out - delete our node
break;
}
@@ -263,16 +261,23 @@ public class LockInternals {
}
} catch (Exception e) {
ThreadUtils.checkInterrupted(e);
- doDelete = true;
+ deleteOurPathQuietly(ourPath, e);
throw e;
- } finally {
- if (doDelete) {
- deleteOurPath(ourPath);
- }
+ }
+ if (!haveTheLock) {
+ deleteOurPath(ourPath);
}
return haveTheLock;
}
+ private void deleteOurPathQuietly(String ourPath, Exception ex) {
+ try {
+ deleteOurPath(ourPath);
+ } catch (Exception suppressed) {
+ ex.addSuppressed(suppressed);
+ }
+ }
+
private void deleteOurPath(String ourPath) throws Exception {
try {
client.delete().guaranteed().forPath(ourPath);
diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/locks/TestLockCleanlinessWithFaults.java b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/locks/TestLockInternals.java
similarity index 62%
rename from curator-recipes/src/test/java/org/apache/curator/framework/recipes/locks/TestLockCleanlinessWithFaults.java
rename to curator-recipes/src/test/java/org/apache/curator/framework/recipes/locks/TestLockInternals.java
index d1f28500..f96a791a 100644
--- a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/locks/TestLockCleanlinessWithFaults.java
+++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/locks/TestLockInternals.java
@@ -30,9 +30,9 @@ import org.apache.curator.test.BaseClassForTests;
import org.apache.zookeeper.KeeperException;
import org.junit.jupiter.api.Test;
-public class TestLockCleanlinessWithFaults extends BaseClassForTests {
+public class TestLockInternals extends BaseClassForTests {
@Test
- public void testNodeDeleted() throws Exception {
+ public void testNodeDeletedCleanlyWithFaults() throws Exception {
final String PATH = "/foo/bar";
CuratorFramework client = null;
@@ -62,4 +62,38 @@ public class TestLockCleanlinessWithFaults extends BaseClassForTests {
TestCleanState.closeAndTestClean(client);
}
}
+
+ @Test
+ public void testAttemptLockFailedException() throws Exception {
+ final String PATH = "/foo/bar";
+ try (CuratorFramework client = CuratorFrameworkFactory.builder()
+ .connectString(server.getConnectString())
+ .connectionTimeoutMs(1000)
+ .retryPolicy(new RetryNTimes(0, 0))
+ .build()) {
+ client.start();
+
+ client.create().creatingParentsIfNeeded().forPath(PATH);
+ assertEquals(client.checkExists().forPath(PATH).getNumChildren(), 0);
+
+ LockInternals internals = new LockInternals(client, new StandardLockInternalsDriver(), PATH, "lock-", 1) {
+ @Override
+ List<String> getSortedChildren() throws Exception {
+ closeServer();
+ getClient()
+ .getZookeeperClient()
+ .getZooKeeper()
+ .getTestable()
+ .injectSessionExpiration();
+ throw new KeeperException.NoNodeException();
+ }
+ };
+ try {
+ internals.attemptLock(0, null, null);
+ fail("expect no node");
+ } catch (KeeperException.NoNodeException ex) {
+ // expected
+ }
+ }
+ }
}
diff --git a/curator-test/src/main/java/org/apache/curator/test/BaseClassForTests.java b/curator-test/src/main/java/org/apache/curator/test/BaseClassForTests.java
index 15bd715a..76a87ad8 100644
--- a/curator-test/src/main/java/org/apache/curator/test/BaseClassForTests.java
+++ b/curator-test/src/main/java/org/apache/curator/test/BaseClassForTests.java
@@ -131,7 +131,7 @@ public class BaseClassForTests {
closeServer();
}
- private void closeServer() {
+ protected void closeServer() {
if (server != null) {
try {
server.close();