Avoid collecting potential large directory list

2 files changed, 15 insertions, 14 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java
index 4e55e464fa7..6c1c1a48a9c 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java
@@ -20,10 +20,8 @@ import java.nio.file.attribute.PosixFilePermissions;
 import java.nio.file.attribute.UserPrincipal;
 import java.nio.file.attribute.UserPrincipalLookupService;
 import java.time.Instant;
-import java.util.List;
 import java.util.Optional;
 import java.util.Set;
-import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import static com.yahoo.vespa.hosted.node.admin.task.util.file.IOExceptionUtil.ifExists;
@@ -231,8 +229,8 @@ public class UnixPath {
      */
     public boolean deleteRecursively() {
         if (!isSymbolicLink() && isDirectory()) {
-            for (UnixPath path : listContentsOfDirectory()) {
-                path.deleteRecursively();
+            try (Stream<UnixPath> paths = listContentsOfDirectory()) {
+                paths.forEach(UnixPath::deleteRecursively);
             }
         }
         return uncheck(() -> Files.deleteIfExists(path));
@@ -243,13 +241,14 @@ public class UnixPath {
         return this;
     }
 
-    public List<UnixPath> listContentsOfDirectory() {
-        try (Stream<Path> stream = Files.list(path)){
-            return stream
-                    .map(UnixPath::new)
-                    .collect(Collectors.toList());
+    /** Lists the contents of this as a stream. Callers should use try-with to ensure that the stream is closed */
+    public Stream<UnixPath> listContentsOfDirectory() {
+        try {
+            // Avoid the temptation to collect the stream here as collecting a directory with a high number of entries
+            // can quickly lead to out of memory conditions
+            return Files.list(path).map(UnixPath::new);
         } catch (NoSuchFileException ignored) {
-            return List.of();
+            return Stream.empty();
         } catch (IOException e) {
             throw new RuntimeException("Failed to list contents of directory " + path.toAbsolutePath(), e);
         }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java
index b420b6aac23..e5c2e35f2b2 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java
@@ -27,6 +27,7 @@ import java.util.Optional;
 import java.util.Set;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static com.yahoo.yolean.Exceptions.uncheck;
 import static org.junit.Assert.assertEquals;
@@ -247,10 +248,11 @@ public class CoredumpHandlerTest {
 
     private static void assertFolderContents(Path pathToFolder, String... filenames) {
         Set<String> expectedContentsOfFolder = Set.of(filenames);
-        Set<String> actualContentsOfFolder = new UnixPath(pathToFolder)
-                .listContentsOfDirectory().stream()
-                .map(unixPath -> unixPath.toPath().getFileName().toString())
-                .collect(Collectors.toSet());
+        Set<String> actualContentsOfFolder;
+        try (Stream<UnixPath> paths = new UnixPath(pathToFolder).listContentsOfDirectory()) {
+            actualContentsOfFolder = paths.map(unixPath -> unixPath.toPath().getFileName().toString())
+                                          .collect(Collectors.toSet());
+        }
         assertEquals(expectedContentsOfFolder, actualContentsOfFolder);
     }