Merge pull request #892 from tgodzik/fix-file-leaks

Fix not closing streams when using Files.list
scalameta · Sep 2, 2019 · cc9eadb · cc9eadb
2 parents 1d058b4 + 0aafd10
commit cc9eadb
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 24 deletions.
diff --git a/metals/src/main/scala/scala/meta/internal/builds/GradleDigest.scala b/metals/src/main/scala/scala/meta/internal/builds/GradleDigest.scala
@@ -2,11 +2,8 @@ package scala.meta.internal.builds
 
 import scala.meta.io.AbsolutePath
 import scala.meta.internal.mtags.WalkFiles
-import scala.meta.internal.jdk.CollectionConverters._
 import java.security.MessageDigest
-import java.nio.file.Files
-import java.util.stream.Collectors
-import java.nio.file.Path
+import scala.meta.internal.mtags.ListFiles
 
 object GradleDigest extends Digestable {
   override protected def digestWorkspace(
@@ -36,25 +33,21 @@ object GradleDigest extends Digestable {
       workspace: AbsolutePath,
       digest: MessageDigest
   ): Boolean = {
-    val (subprojects, dirs) = Files
-      .list(workspace.toNIO)
-      .filter(Files.isDirectory(_))
-      .collect(Collectors.toList[Path])
-      .asScala
-      .partition { file =>
-        Files
-          .list(file)
-          .anyMatch { path =>
-            val stringPath = path.toString
-            stringPath.endsWith(".gradle") || stringPath.endsWith("gradle.kts")
-          }
-      }
+    val directories = ListFiles(workspace).filter(_.isDirectory)
+
+    val (subprojects, dirs) = directories.partition { file =>
+      ListFiles
+        .exists(file) { path =>
+          val stringPath = path.toString
+          stringPath.endsWith(".gradle") || stringPath.endsWith("gradle.kts")
+        }
+    }
     /*
        If a dir contains a gradle file we need to treat is as a workspace
      */
     val isSuccessful = subprojects.forall { file =>
       digestWorkspace(
-        AbsolutePath(file),
+        file,
         digest
       )
     }
@@ -63,7 +56,7 @@ object GradleDigest extends Digestable {
        If it's a dir we need to keep searching since gradle can have non trivial workspace layouts
      */
     isSuccessful && dirs.forall { file =>
-      digestSubProjects(AbsolutePath(file), digest)
+      digestSubProjects(file, digest)
     }
   }
 }
diff --git a/metals/src/main/scala/scala/meta/internal/builds/SbtDigest.scala b/metals/src/main/scala/scala/meta/internal/builds/SbtDigest.scala
@@ -27,9 +27,7 @@ object SbtDigest extends Digestable {
     if (!path.isDirectory) {
       true
     } else {
-      var success = true
-      ListFiles.foreach(path)(file => success &= digestSbtFile(digest)(file))
-      success
+      ListFiles.forall(path)(file => digestSbtFile(digest)(file))
     }
   }
 

diff --git a/mtags/src/main/scala/scala/meta/internal/mtags/ListFiles.scala b/mtags/src/main/scala/scala/meta/internal/mtags/ListFiles.scala
@@ -3,17 +3,37 @@ package scala.meta.internal.mtags
 import java.nio.file.Files
 import scala.meta.io.AbsolutePath
 import scala.collection.mutable
+import java.util.stream.{Stream => JStream}
+import java.nio.file.Path
 
 object ListFiles {
   def apply(root: AbsolutePath): mutable.ArrayBuffer[AbsolutePath] = {
     val buf = mutable.ArrayBuffer.empty[AbsolutePath]
     foreach(root)(file => buf += file)
     buf
   }
-  def foreach(root: AbsolutePath)(fn: AbsolutePath => Unit): Unit = {
+
+  def exists(root: AbsolutePath)(fn: AbsolutePath => Boolean) =
+    closing(root) {
+      _.anyMatch(file => fn(AbsolutePath(file)))
+    }
+
+  def forall(root: AbsolutePath)(fn: AbsolutePath => Boolean) =
+    closing(root) {
+      _.allMatch(file => fn(AbsolutePath(file)))
+    }
+
+  def foreach(root: AbsolutePath)(fn: AbsolutePath => Unit) =
+    closing(root) {
+      _.forEach(file => fn(AbsolutePath(file)))
+    }
+
+  private def closing[T](
+      root: AbsolutePath
+  )(fn: JStream[Path] => T): T = {
     val ls = Files.list(root.toNIO)
     try {
-      ls.forEach(file => fn(AbsolutePath(file)))
+      fn(ls)
     } finally {
       ls.close()
     }