Revisiting MayHoldCloseableResource

[Joe Bowbeer]

Agree.

Though [need not do so] seems awkward below:

"... but those applying only to sets of classes that do not hold releasable
resources [need not do so]."

Try "do not need to"?


On Mon, Aug 19, 2013 at 5:07 PM, Doug Lea <dl at cs.oswego.edu> wrote:

> On 08/15/2013 04:25 PM, Brian Goetz wrote:
>
>> Last chance for comment.
>>
>
> Sorry for delay. (Swamped...)
>
> My sense is that your AutpCloseable update could still do better in
> separating the underlying issue (that an object may hold resources)
> from the usage guidance ("must close" or not.) Here's a shot at it:
>
>
> /**
>  * An object that may hold resources (such as file or socket handles)
>  * until it is closed. The {@link #close()} method is called
>  * automatically when exiting a {@code try}-with-resources block for
>  * which the resource has been declared in the header. This
>  * construction ensures prompt release of resources, avoiding resource
>  * exhaustion exceptions and errors that may otherwise occur.
>  *
>  * @apiNote
>  * <p>It is possible, and in fact common, for a base class to
>  * implement AutoCloseable even though not all of its subclasses
>  * (perhaps only a few) hold releasable resources. It is best practice
>  * for usages applying to <em>any</em> possible subclass to use {@code
>  * try}-with-resources constructions, but those applying only to sets
>  * of classes that do not hold releasable resources need not do so.
>  * In frameworks such as {@link java.util.Stream}, that support both
>  * IO-based and non-IO-based forms, it is unnecessary to use {@code
>  * try}-with-resources blocks in methods that will never be applied to
>  * IO-based forms.
>  *
>  * @author Josh Bloch
>  * @since 1.7
>  */
> @FunctionalInterface
> public interface AutoCloseable {
>
>
>
>
>> On 8/13/2013 1:03 PM, Brian Goetz wrote:
>>
>>> OK, here's proposed spec:
>>>
>>> For AC:
>>>
>>>   /**
>>> - * A resource that must be closed when it is no longer needed.
>>> + * A resource that may need to be closed when it is no longer needed,
>>> and whose
>>> + * {@link #close()} method will be called automatically when exiting a
>>> + * {@code try}-with-resources block for which the resource has been
>>> declared in
>>> + * the header.  A resource would need to be explicitly closed if it
>>> holds a
>>> + * scarce system resource that is not promptly reclaimed by garbage
>>> collection,
>>> + * such as a file or socket handle.
>>> + *
>>> + * @apiNote
>>> + * <p>It should generally be possible to determine statically whether a
>>> resource
>>> + * that may need to be closed actually does need to be closed (in which
>>> case one
>>> + * should consider using try-with-resources to ensure its timely
>>> closure).
>>> + * For example, most {@link InputStream} implementations need to be
>>> closed,
>>> + * though {@link ByteArrayInputStream} does not need to be; most {@link
>>> Stream}
>>> + * implementations do <em>not</em> need to be closed, but streams which
>>> encapsulate
>>> + * IO resources, like those produced by the static factories in {@link
>>> Files}
>>> + * such as {@link Files#walk(Path, FileVisitOption...)}, do need to be
>>> closed.
>>>
>>> For BaseStream and friends:
>>>
>>> + * <p>Streams have a {@link #close()} method and implement {@link
>>> 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 {@link 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 {@code try}-with-resources statement.)
>>> + *
>>>
>>> For Files.walk and friends:
>>>
>>> +     * <p>The returned stream encapsulates one or more {@link
>>> DirectoryStream}s.
>>> +     * If timely disposal of file system resources is required, the
>>> +     * {@code try}-with-resources construct should be used to ensure
>>> that the
>>> +     * stream's {@link Stream#close close} method is invoked after the
>>> stream
>>> +     * operations are completed.
>>>
>>>
>>> On 8/12/2013 1:43 PM, Brian Goetz wrote:
>>>
>>>> After discussion both in the EG and within Oracle, here's where I think
>>>> this should land:
>>>>
>>>>   - Eliminate MayHoldCloseableResource, which is fundamentally flawed
>>>> unless the spec for AutoCloseable is fixed, in which case it becomes
>>>> unnecessary.
>>>>   - Fix the spec for AutoCloseable.
>>>>   - Eliminate @HoldsResource, not because it is intrinsically bad, but
>>>> because it is a half solution and we should work towards a complete one
>>>> for 9, likely using something like JSR-308 checkers.  The Eclipse
>>>> inspector can be adjusted to work with the new APIs.
>>>>   - Make BaseStream implement AutoCloseable, with additional verbiage
>>>> regarding closing.
>>>>   - Add additional spec to Files.{walk,lines,etc}, and to combinators
>>>> like concat, regarding their close behavior.
>>>>
>>>> The thing that pushed me over the edge from the various "work around the
>>>> problem" solutions is the existence of flatMap:
>>>>
>>>>    Files.walk(dir)
>>>>         .flatMap(Files::lines)
>>>>         ...
>>>>
>>>> where each entry resulting from walk() will turn into a stream creation
>>>> and traversal within the flatMap implementation, and flatMap itself
>>>> needs a way to ensure it is deterministically closed.  This pushes
>>>> strongly for having a close() method on Stream, which, if you're going
>>>> to have that, you might as well be AutoCloseable to make life easier for
>>>> users.
>>>>
>>>> The sole challenge here is to keep people from thinking that they *need*
>>>> to close all streams, and therefore mangle their code in bad ways to
>>>> satisfy this perceived need.
>>>>
>>>>
>>>> On 8/2/2013 7:39 PM, Brian Goetz wrote:
>>>>
>>>>> There seems to be consensus that the current strawman for
>>>>> MayHoldCloseableResoruce and @HoldsResource is too flawed to proceed.
>>>>>
>>>>> Flaw #1 relates to @HoldsResource.  While there seems to be consensus
>>>>> that annotations are reasonable for imparting information like this
>>>>> which is suited to static analysis tools, the current proposal (a)
>>>>> confusingly leaks through into subclass Javadoc and (b) seems too
>>>>> weak.  Conclusion: just dropping @HR from the current scheme (leaving
>>>>> only MHCR <: AC) is an improvement; we can revisit what the correct
>>>>> annotations for hinting at resourceful-ness later.
>>>>>
>>>>> Flaw #2 relates to MHCR.  The real problem is that AutoCloseable is
>>>>> overspecified; it says "a resource that must be closed."  This is an
>>>>> overreach; what j.l.AC should say is "has a close method which is
>>>>> called automatically on exit from a TWR block."  Because of this
>>>>> overspecify, subclassing doesn't help, because if an AC must be
>>>>> closed, so must a MHCR.  Conclusion: dropping MHCR and just having
>>>>> BaseStream <: AC is an improvement.
>>>>>
>>>>> Flaw #3 relates to the spec of AC.  The conclusion of the core library
>>>>> maintainers is that it is acceptable to correct the spec, and replace
>>>>> the "must" with "may".  This leaves us with a fixed AC, and BaseStream
>>>>> <: AC.  This seems strictly better than where we are now.  Let's call
>>>>> this "Option 1."
>>>>>
>>>>> But, it is possible we might still choose to punt further for now.
>>>>>
>>>>> 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).
>>>>>
>>>>> For other cases, like BufferedReader.lines(), the BR itself is AC, so
>>>>> users can handle resource management like:
>>>>>
>>>>>    try (BufferedReader b = ...) {
>>>>>       b.lines()...
>>>>>    }
>>>>>
>>>>> and there is no need to close the *stream* at all, since the user has
>>>>> access to the actual closeable resource.  It is only the static
>>>>> methods such as Files.list(dir), which don't provide the user access
>>>>> in this way.  The methods we provide are:
>>>>>
>>>>> Files.list(Path)
>>>>> Files.walk(Path, maxDepth, options)
>>>>> Files.walk(Path, options)
>>>>> Files.find(Path, maxDepth, BiPredicate<Path, BasicFileAttributes>,
>>>>> options)
>>>>> Files.lines(Path)
>>>>>
>>>>> Colin @ Google pointed out that if the spliterators for these streams
>>>>> were lazy (didn't open the resource until the terminal began), and all
>>>>> terminal ops called the close hook in a finally block after
>>>>> processing, then we wouldn't need to be AC, since there would be no
>>>>> way to leak a closeable resource.  (This would move resource-not-found
>>>>> exceptions from the factory method to the terminal op.)
>>>>>
>>>>> Still, having a close() method and *not* being AC may look weird to
>>>>> the users.
>>>>>
>>>>> Let's call this "Option 2": have a close() method, don't extend AC,
>>>>> make sure that all terminals call close, make sure that all static
>>>>> resource-holding stream factories produce late-binding spliterators.
>>>>>
>>>>> Option 3 would be to simply get rid of these methods (note that
>>>>> Files.list and Files.lines are pretty simple wrappers around existing
>>>>> AC abstractions, DirectoryStream and BufferedReader.)
>>>>>
>>>>> Orthogonal to Options 2 and 3 would be fixing the spec of AC.
>>>>>
>>>>>
>>>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/lambda-libs-spec-experts/attachments/20130819/7f9128bf/attachment-0001.html