Wednesday, 18 February 2015

Java 8 pitfall - Beware of Files.lines()

There's a really nice new feature in Java8 which allows you to get a stream of Strings from a file in a one liner.

List lines = Files.lines(path).collect(Collectors.toList());

You can manipulate the Stream as you would with any other Stream for example you might want to filter() or map() or limit() or skip() etc.

I started using this all over my code until I was hit with this exception,

Caused by: java.nio.file.FileSystemException: /tmp/date.txt: Too many open files in system
at sun.nio.fs.UnixException.translateToIOException(UnixException.java:91)
at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)
at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)
at sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:214)
at java.nio.file.Files.newByteChannel(Files.java:361)
at java.nio.file.Files.newByteChannel(Files.java:407)
at java.nio.file.spi.FileSystemProvider.newInputStream(FileSystemProvider.java:384)
at java.nio.file.Files.newInputStream(Files.java:152)
at java.nio.file.Files.newBufferedReader(Files.java:2784)
at java.nio.file.Files.lines(Files.java:3744)
at java.nio.file.Files.lines(Files.java:3785) 

For some reason I had too many open files! Odd, doesn't Files.lines() close the file?

See code below (run3()) where I've created reproduced the issue:


My code looked something like run3() which produced the exception. I proved this by running the unix command lsof (lists open files) and noticing many many instances of date.txt open. To check that the problem was indeed with Files.lines() I made sure that the code ran with run1() using a BufferedReader, which it did.  By reading through the source code for Files I realised that the Stream need to be created in an auto closable.  When I implemented that in run2() the code ran fine again.

In my opinion I don't think that this is not particularly intuitive.  It really spoils the one liner when you have to use the auto closable.  I guess that the code does need a signal as to when to close the file but somehow it would be nice if that was hidden from us.  At the very least it should be highlighted in the JavaDoc which it is not :-) 

5 comments:

  1. The need to use try-with-resources is mentioned (without stressing its importance) in the javadoc for Files.lines(Path, Charset)

    ReplyDelete
  2. It does indeed but not mentioned at all Files.lines(Path) which the method I would expect most people will be looking at in the first instance.

    ReplyDelete
  3. Quote from the Stream javadoc:

    Streams have a BaseStream.close() method and implement AutoCloseable, but nearly all stream instances do not actually need to be closed after use. Generally, only streams whose source is an IO channel (such as those returned by Files.lines(Path, Charset)) will require closing. Most streams are backed by collections, arrays, or generating functions, which require no special resource management. (If a stream does require closing, it can be declared as a resource in a try-with-resources statement.)

    ReplyDelete
    Replies
    1. Thanks for pointing this out - does show the value of reading all the JavaDoc carefully. Btw I would add that readAllLines() will close without being in a try-with-resource block.

      Delete
  4. I've filed and fixed https://bugs.openjdk.java.net/browse/JDK-8073923 which should improve the documentation issues that were mentioned here.

    ReplyDelete