[CURATOR-430] deletingChildrenIfNeeded maybe cannot delete children completely when multi-client delete concurrently

curator-client/src/main/java/org/apache/curator/utils/ZKPaths.java

@@ -313,7 +313,13 @@ public static void deleteChildren(ZooKeeper zookeeper, String path, boolean dele
     {
         PathUtils.validatePath(path);
 
-        List<String> children = zookeeper.getChildren(path, null);
+        List<String> children;
+        try {
+            children = zookeeper.getChildren(path, null);
+        } catch (KeeperException.NoNodeException e) {
+            // someone else has deleted the node it since we checked
+            return;
+        }
         for ( String child : children )
         {
             String fullPath = makePath(path, child);

curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkEdges.java

@@ -612,4 +612,57 @@ public void testStopped() throws Exception
             // correct
         }
     }
+
+    @Test
+    public void testDeleteChildrenConcurrently() throws Exception {
+        final CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1));
+        CuratorFramework client2 = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1));
+        try {
+            client.start();
+            client.getZookeeperClient().blockUntilConnectedOrTimedOut();
+            client2.start();
+            client2.getZookeeperClient().blockUntilConnectedOrTimedOut();
+
+            int childCount = 5000;
+            for (int i = 0; i < childCount; i++) {
+                client.create().creatingParentsIfNeeded().forPath("/parent/child" + i);
+            }
+
+            final CountDownLatch countDownLatch = new CountDownLatch(1);
+            new Thread(new Runnable() {
+                @Override
+                public void run() {
+                    try {
+                        client.delete().deletingChildrenIfNeeded().forPath("/parent");
+                    } catch (Exception e) {
+                        if (e instanceof KeeperException.NoNodeException) {
+                            Assert.fail("client delete failed, shouldn't throw NoNodeException", e);
+                        } else {
+                            Assert.fail("unknown exception", e);
+                        }
+                    } finally {
+                        countDownLatch.countDown();
+                    }
+                }
+            }).start();
+
+            Thread.sleep(20L);
+            try {
+                client2.delete().forPath("/parent/child" + (childCount/2));
+            } catch (Exception e) {
+                if (e instanceof KeeperException.NoNodeException) {
+                    Assert.fail("client2 delete failed, shouldn't throw NoNodeException", e);
+                } else {
+                    Assert.fail("unknown exception", e);
+                }
+            }
+
+            Assert.assertTrue(countDownLatch.await(10, TimeUnit.SECONDS));
+
+            Assert.assertNull(client2.checkExists().forPath("/parent"));
+        } finally {
+            CloseableUtils.closeQuietly(client);
+            CloseableUtils.closeQuietly(client2);
+        }
+    }
 }