Why stream from BufferedReader::lines is not closing the reader?

[Brian Goetz]

Gernot's answer is exactly correct.

Think of it this way: do you expect BufferedReader.readLine() to close 
the stream when it is at the EOF?  Because that's all lines() does -- 
repeatedly call readLine() for you.  If you opened the buffered reader, 
then try-with-resources is what you want.

But, all is not lost.  Based on your complaint, my guess is that you're 
wondering what to do in a case where you open the BR as part of a 
flatMap operation, like "enumerate the files, then for each file, 
enumerate the lines".  Here, we support the rule of "whoever opens the 
stream, closes the stream" by closing the stream that is returned by a 
flatMapper after iterating it.  Which means that, if your stream holds 
non-memory resources, the flatMapper should create a stream that closes 
the underlying stream, like:

   blah.flatMap(path -> {
      BufferedReader br = new BufferedReader(path);
      return br.lines.onClose(br::close);
   }
   ...

Now, this is not terrible, but a little roundabout.  We anticipated this 
would be a common case and created static methods in Files which create 
streams that are already set up with close handlers.  So you could do:

try {
   Files.walk(dir)
        .flatMap(Files::lines)
        ...

and everything will work as you expect.

Actually, to be totally safe, you should also handle the closing of the 
top-level stream:

   try (Stream<Path> paths = Files.walk(dir)) {
       paths.flatMap(Files::lines)...
   }

To summarize, flatMap() is the only operation that internally closes the 
stream after its done, and for good reason -- it is the only case where 
the stream is effectively opened by the operation itself, and therefore 
should be closed by the operation too.  Any other streams are assumed to 
be opened by the caller, and therefore should be closed by the caller.

On 11/18/2013 7:42 AM, Tomasz Kowalczewski wrote:
> Hi,
>
> thanks for your answers.
>
> Typically I would agree with Gernot that "Whoever creates a Resource is
> responsible for closing it.". My example here might have been little
> misleading in this regard.
> Reader close() method closes the InputStream that was passed to it (thus
> violating this principle) and what I really wanted is to close the Reader
> (which in turns closes the input stream).
>
> I still think that it would be more user friendly if classes dealing with
> I/O should indicate if and when they (don't) close the stream.
>
> As for Alan's answer:
>
> My motivation was following use case: I am downloading files from remote
> service and flatMapping them into lines they contain. Creation of the
> Reader was hidden inside the mapper. My code does exactly what you
> suggested with wrapping the ::close call and throwing UncheckedIOException.
>
> I hoped to get some guidance in how can I reduce boilerplate even further.
> As I see it now everyone will be writing such wrappers themselves.
>
> It is not to say that I don't greatly appreciate all the hard work of all
> people involved in lambdification of Java. It's fantastic work you have
> done.
>
>
>
>
> On Mon, Nov 18, 2013 at 11:10 AM, Alan Bateman <Alan.Bateman at oracle.com>wrote:
>
>> On 18/11/2013 08:46, Tomasz Kowalczewski wrote:
>>
>>> 3. If I want the stream to be closed I need to do:
>>>
>>> reader.lines().onClose(is::close).doStreamyStuff().collect(...);
>>>
>>> where *is* is an underlying InputStream.
>>>
>>> The problem with this code is that ::close throws IOException which is not
>>> compatible with Runnable accepted by onClose(); Is there a better way?
>>> Some
>>> wrapper I can use to inject a call to close?
>>>
>>>   As you have a reference to the buffered reader it might be simplest to
>> use try-with-resources so that the reader (and hence the underlying input
>> stream) will be closed, something like:
>>
>> try (BufferReader reader = ...) {
>>    reader.lines().doStreamyStuff().collect(...);
>> }
>>
>> Alternatively, if the reader is to a file then you can use Files.lines as
>> it returns a Stream that will arrange to close the underlying file when you
>> close the stream.
>>
>> Otherwise if you really need to really want the stream close to close the
>> underlying input stream (or reader more likely) then the runnable will need
>> to translate the checked IOException to an unchecked exception. The new
>> java.io.UncheckedIOException is probably what you want.
>>
>> -Alan.
>>
>>
>>
>>
>>
>>
>>
>>
>
>