Resource leak : scala.io.BufferedSource not closed

[michelou]

minimized code

I discovered the issue when running the REPL test LoadTests.scala on Windows (but the issue may not be specific to Windows).

More precisely the following line (line 76 in LoadTests.scala) helped me to identify the issue:

Files.walk(dir).sorted(Comparator.reverseOrder).forEach(Files.delete)

NB. This is another case where an issue gets detected when testing on Windows but falls through the net on Unix/MacOS.

Compilation output

$ sbt -sbt-version 1.3.8 "testOnly dotty.tools.repl.LoadTests"
[info] Loading settings for project dotty-multiline-build-build from build.sbt ...
[...]
[info] Test run started
[info] Test dotty.tools.repl.LoadTests.helloworld started
[info] Test dotty.tools.repl.LoadTests.maindefs started
[info] Test dotty.tools.repl.LoadTests.maindef started
[error] Test dotty.tools.repl.LoadTests failed: java.nio.file.FileSystemException: %TEMP%\repl_load_src3584160550809821314\repl_test3713211733288201754.scala: The process cannot access the file because it is being used by another process.
[error] , took 0.0 sec
[error]     at sun.nio.fs.WindowsException.translateToIOException(WindowsException.java:86)
[error]     at sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:97)
[error]     at sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:102)
[error]     at sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:269)
[error]     at sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:103)
[error]     at java.nio.file.Files.delete(Files.java:1126)
[error]     at dotty.tools.repl.LoadTests$.removeDir$$anonfun$1(LoadTests.scala:77)
[error]     at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
[error]     at java.util.ArrayList.forEach(ArrayList.java:1257)
[error]     at java.util.stream.SortedOps$RefSortingSink.end(SortedOps.java:395)
[error]     at java.util.stream.Sink$ChainedReference.end(Sink.java:258)
[error]     at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:483)
[error]     at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:472)
[error]     at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
[error]     at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
[error]     at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
[error]     at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:485)
[error]     at dotty.tools.repl.LoadTests$.removeDir(LoadTests.scala:77)
[error]     at dotty.tools.repl.LoadTests.removeDir(LoadTests.scala)
[error]     ...
[info] Test run finished: 1 failed, 0 ignored, 3 total, 5.563s
[error] Failed: Total 4, Failed 1, Errors 0, Passed 3
[error] Failed tests:
[error]         dotty.tools.repl.LoadTests
[error] (dotty-compiler / Test / testOnly) sbt.TestsFailedException: Tests unsuccessful
[error] Total time: 16 s, completed 22 Feb 2020 15:19:33

Expectation

$ sbt -sbt-version 1.3.8 "compile; testOnly dotty.tools.repl.LoadTests"
[info] Loading settings for project dotty-multiline-build-build from build.sbt ...
[...]
[info] Test run started
[info] Test dotty.tools.repl.LoadTests.helloworld started
[info] Test dotty.tools.repl.LoadTests.maindefs started
[info] Test dotty.tools.repl.LoadTests.maindef started
[info] Test run finished: 0 failed, 0 ignored, 3 total, 5.828s
[info] Passed: Total 3, Failed 0, Errors 0, Passed 3
[success] Total time: 14 s, completed 22 Feb 2020 15:24:49

Solution

The offending code is located in file ReplDriver.scala and looks as follows (line 350):

val contents = scala.io.Source.fromFile(file, "UTF-8").mkString

Method fromFile returns an BufferedSource object which needs to be closed ! The following code solves the issue for me:

val source = scala.io.Source.fromFile(file, "UTF-8")
val contents = source.mkString
source.close()

NB. Class Source does implement the Java interface Closeable, not the object Source.

Discussion

Usage of Source.fromFile can currently be found in 6 locations in Dotty, namely:

$ find.exe compiler -name "*.scala" -exec grep -H "Source.fromFile" \{} ;
compiler/src/dotty/tools/repl/ReplDriver.scala:        val contents = scala.io.Source.fromFile(file, "UTF-8").mkString
compiler/test/debug/Gen.scala:    val lines = Source.fromFile(file).getLines().toBuffer
compiler/test/dotty/tools/repl/ReplTest.scala:    val lines = Source.fromFile(f, "UTF-8").getLines().buffered
compiler/test/dotty/tools/repl/ReplTest.scala:      Source.fromFile(f, "UTF-8").getLines().flatMap(filterEmpties).mkString(EOL)
compiler/test/dotty/tools/vulpix/FileDiff.scala:      else Source.fromFile(checkFile, "UTF-8").getLines().toList
compiler/test/dotty/tools/vulpix/ParallelTesting.scala:        Source.fromFile(file, "UTF-8").getLines().zipWithIndex.foreach { case (line, lineNbr) =>

I have a PR pending to fix ReplDriver in order to successfully pass the REPL test LoadTests .

Let me know

• if the case needs more investigation (resp. should be transmitted to the Scala team too)
• or the fix should be applied elsewhere.

[smarter]

Thanks for the investigation! Adding .close() everywhere sounds good to me. Java 11 added a convenient Files.readString method but we can't use that yet.

if the case needs more investigation (resp. should be transmitted to the Scala team too)

Looks like there's one place that uses Source.fromFile in the scala/scala repo which could benefit from being closed too, feel free to open an issue there too.