Revisiting MayHoldCloseableResource

[Victor Nazarov]

Brian,

I just like to show my support for Option 2. It seems the clearest solution
to me, since all basic use cases will be covered and will be handled with
deterministic and correct resource management.


Victor Nazarov


On Sun, Aug 4, 2013 at 8:48 AM, Jim Mayer <jim at pentastich.org> wrote:

> Brian,
>
> Does option one include the "make sure that all terminals call close, make
> sure that all static resource-holding stream factories produce late-binding
> spliterators" part of option two?
>
> I think those two changes are critically important to avoid resource leaks.
>
> So, if option one does not include the changes then I would prefer option
> two followed by option three.
>
> If option one does include this changes,  then I'm not sure.  Do you have a
> good example of where having Stream be AutoCloseable really helps?  I ask
> because forms such as "try(new FileStream(...).lines()...stuff...) {...}"
> give a false sense of security.  There is no point in using try/finally
> unless avoiding resource leaks is important, but the "stuff" can still
> throw an exception and that would cause the FileStream to leak.
>
> Jim
> On Aug 3, 2013 1:15 PM, "Brian Goetz" <brian.goetz at oracle.com> wrote:
>
> > >> Observation: the need for Stream to have a close() method comes about
> > >> because of the desire to have static factory methods that will create
> > >> streams that encapsulate a resource that requires closing, when there
> is
> > >> no way to get access to that underlying resource.  Such as
> > Files.walk(dir)
> > >> or Files.lines(path).
> > >>
> > >
> > > Or rather, no way provided in JDK. What if these methods were
> > > recast do so? That is, exposing some closeable thing.
> > > Maybe something like Files.directoryStream(dir).files().
> > >
> > > A little less convenient, but avoids lots of problems.
> >
> > This was my thinking at first too, but don't forget about cases like
> > flatMap, where the stream is created inside a pipeline.  Let's say we do
> as
> > you suggest for dirStream.  But what if you want "all the lines in all
> > those files"?  You'd flatMap them to a stream of strings:
> >
> > try (dirStream = ...) {
> >   dirStream.stream()
> >                     .flatMap(path -> Files.lines(path))
> >                     ...
> > }
> >
> > There's no place to put TWR around the lines() stream creation even if
> you
> > expand it as you suggest.
> >
> > This (reasonable!) use case makes me hesitant to solve the problem simply
> > by pretending it didn't occur to us to provide static methods here.
> >
> >
> >
> >
>