Some Classes with a public void close() don't implement AutoCloseable
After failing to wrap a XMLStreamReader in a try-with-resources after discovering it's close() method, I thought about checking what other classes have a public void close() method in the JDK but don't implement AutoCloseable. So I wrote a small program that enumerates all classes present in in the jrt image with a public void close() method that does not directly or indirectly implement AutoCloseable: https://gist.github.com/DasBrain/8d50e02e35654870e2c2c00bf3f79954 (Output & Code, Code requires JDK 14 with preview features and ASM) As I am not an expert in any of those areas where classes showed up, and I don't know if it was an oversight, or if there is a good reason for not implementing AutoCloseable. Implementing AutoCloseable is a double edged sword, a few IDEs warn about resource leaks based on that interface. And treat an AutoCloseable passed to methods who return an AutoCloseable as the return value wraping the parameter. I did look at a few classes: * javax.xml.stream.XMLStreamReader:Javadoc: "Frees any resources associated with this Reader. This method does not close the underlying input source." Better not implement it? * javax.naming.Context: Javadoc: "This method releases this context's resources immediately, instead of waiting for them to be released automatically by the garbage collector." Implement AutoCloseable? * java.util.logging.Handler: Lifecycle seems to be managed by a LogManager. Better not implement AutoCloseable? * jdk.internal.jrtfs.ExplodedImage: class is package private, overrides close() from SystemImage while increasing visibility to public. I did not restrict my enumeration to public and exported types - it was easier not to do that and I'm lazy. This touches many unrelated areas, but David Holmes suggested that I take it to core-libs-dev. I know, I just did some static code analysis, but it seems that there might be some low hanging fruits. And for public classes I wish to have at least some rationale why a class with a public void close() method doesn't implement AutoCloseable. Thanks, Johannes Classes found: com/sun/jdi/connect/spi/Connection com/sun/jndi/dns/BaseNameClassPairEnumeration com/sun/jndi/dns/DnsClient com/sun/jndi/dns/DnsContext com/sun/jndi/dns/Resolver com/sun/jndi/ldap/AbstractLdapNamingEnumeration com/sun/jndi/ldap/LdapCtx com/sun/jndi/ldap/LdapReferralContext com/sun/jndi/ldap/LdapSchemaCtx com/sun/jndi/ldap/ext/StartTlsResponseImpl com/sun/jndi/rmi/registry/BindingEnumeration com/sun/jndi/rmi/registry/NameClassPairEnumeration com/sun/jndi/rmi/registry/RegistryContext com/sun/jndi/toolkit/dir/ContextEnumerator com/sun/jndi/toolkit/dir/HierMemDirCtx com/sun/jndi/toolkit/dir/HierMemDirCtx$BaseFlatNames com/sun/jndi/toolkit/dir/LazySearchEnumerationImpl com/sun/jndi/toolkit/url/GenericURLContext com/sun/media/sound/AudioFloatFormatConverter$AudioFloatInputStreamChannelMixer com/sun/media/sound/AudioFloatFormatConverter$AudioFloatInputStreamResampler com/sun/media/sound/AudioFloatInputStream com/sun/media/sound/AudioFloatInputStream$BytaArrayAudioFloatInputStream com/sun/media/sound/AudioFloatInputStream$DirectAudioFloatInputStream com/sun/media/sound/ModelAbstractOscillator com/sun/media/sound/ModelDirector com/sun/media/sound/ModelOscillatorStream com/sun/media/sound/ModelStandardDirector com/sun/media/sound/ModelStandardIndexedDirector com/sun/media/sound/RIFFWriter$RandomAccessByteWriter com/sun/media/sound/RIFFWriter$RandomAccessFileWriter com/sun/media/sound/RIFFWriter$RandomAccessWriter com/sun/media/sound/SoftAbstractResampler$ModelAbstractResamplerStream com/sun/media/sound/SoftMainMixer com/sun/media/sound/SoftMixingDataLine$AudioFloatInputStreamResampler com/sun/media/sound/SoftMixingMainMixer com/sun/media/sound/SoftMixingSourceDataLine$NonBlockingFloatInputStream com/sun/naming/internal/VersionHelper$InputStreamEnumeration com/sun/org/apache/xerces/internal/impl/XMLStreamFilterImpl com/sun/org/apache/xerces/internal/impl/XMLStreamReaderImpl com/sun/org/apache/xerces/internal/xinclude/XIncludeTextReader com/sun/org/apache/xml/internal/serializer/EmptySerializer com/sun/org/apache/xml/internal/serializer/SerializationHandler com/sun/org/apache/xml/internal/serializer/SerializerBase com/sun/org/apache/xml/internal/serializer/ToHTMLSAXHandler com/sun/org/apache/xml/internal/serializer/ToUnknownStream com/sun/org/apache/xml/internal/serializer/WriterChain com/sun/tools/javac/api/JavacTaskPool$ReusableContext$ReusableJavaCompiler com/sun/tools/javac/file/JavacFileManager$1 com/sun/tools/javac/file/JavacFileManager$ArchiveContainer com/sun/tools/javac/file/JavacFileManager$Container com/sun/tools/javac/file/JavacFileManager$DirectoryContainer com/sun/tools/javac/file/JavacFileManager$JRTImageContainer com/sun/tools/javac/file/Locations com/sun/tools/javac/main/JavaCompiler com/sun/tools/javac/processing/JavacProcessingEnvironment$DiscoveredProcessors com/sun/tools/javac/processing/JavacProcessingEnvironment$ServiceIterator com/sun/tools/jdi/SharedMemoryConnection com/sun/tools/jdi/SocketConnection com/sun/tools/sjavac/PubApiExtractor com/sun/xml/internal/stream/Entity$ScannedEntity com/sun/xml/internal/stream/XMLEventReaderImpl com/sun/xml/internal/stream/writers/XMLDOMWriterImpl com/sun/xml/internal/stream/writers/XMLEventWriterImpl com/sun/xml/internal/stream/writers/XMLStreamWriterImpl java/awt/SplashScreen java/util/logging/ConsoleHandler java/util/logging/FileHandler java/util/logging/Handler java/util/logging/MemoryHandler java/util/logging/SocketHandler java/util/logging/StreamHandler javax/naming/Context javax/naming/InitialContext javax/naming/NamingEnumeration javax/naming/directory/BasicAttribute$ValuesEnumImpl javax/naming/directory/BasicAttributes$AttrEnumImpl javax/naming/directory/BasicAttributes$IDEnumImpl javax/naming/ldap/StartTlsResponse javax/naming/spi/ContinuationContext javax/smartcardio/CardChannel javax/sql/PooledConnection javax/swing/ProgressMonitor javax/swing/text/rtf/RTFReader$AttributeTrackingDestination javax/swing/text/rtf/RTFReader$ColortblDestination javax/swing/text/rtf/RTFReader$Destination javax/swing/text/rtf/RTFReader$DiscardingDestination javax/swing/text/rtf/RTFReader$FonttblDestination javax/swing/text/rtf/RTFReader$StylesheetDestination javax/swing/text/rtf/RTFReader$StylesheetDestination$StyleDefiningDestination javax/swing/text/rtf/RTFReader$TextHandlingDestination javax/xml/stream/XMLEventReader javax/xml/stream/XMLEventWriter javax/xml/stream/XMLStreamReader javax/xml/stream/XMLStreamWriter javax/xml/stream/util/EventReaderDelegate javax/xml/stream/util/StreamReaderDelegate jdk/internal/jrtfs/ExplodedImage jdk/internal/util/xml/XMLStreamWriter jdk/internal/util/xml/impl/XMLStreamWriterImpl jdk/jfr/internal/consumer/ChunkParser jdk/jshell/SourceCodeAnalysisImpl jdk/tools/jlink/internal/Archive jdk/tools/jlink/internal/DirArchive jdk/tools/jlink/internal/JarArchive jdk/tools/jlink/internal/JmodArchive org/graalvm/compiler/code/CompilationResult org/graalvm/compiler/code/DataSection org/graalvm/compiler/debug/DiagnosticsOutputDirectory org/w3c/dom/html/HTMLDocument sun/awt/image/ImageDecoder sun/jvm/hotspot/debugger/DataSource sun/jvm/hotspot/debugger/InputLexer sun/jvm/hotspot/debugger/MappedByteBufferDataSource sun/jvm/hotspot/debugger/RandomAccessFileDataSource sun/jvm/hotspot/debugger/posix/AddressDataSource sun/jvm/hotspot/debugger/posix/elf/ELFFile sun/jvm/hotspot/debugger/posix/elf/ELFFileParser$ELFFileImpl sun/jvm/hotspot/debugger/win32/coff/COFFFile sun/jvm/hotspot/debugger/win32/coff/COFFFileParser$COFFFileImpl sun/jvm/hotspot/debugger/windbg/AddressDataSource sun/jvm/hotspot/debugger/windbg/DLL sun/net/ProgressSource sun/net/httpserver/ExchangeImpl sun/net/www/URLConnection sun/rmi/log/ReliableLog sun/rmi/runtime/Log$InternalStreamHandler sun/rmi/transport/Connection sun/rmi/transport/tcp/TCPConnection sun/security/smartcardio/ChannelImpl sun/tools/java/ClassPath
Hi Johannes, FYI, back during Project Coin during JDK 7, there were various systematic efforts to review types with a "close" method in the JDK and retrofit AutoCloseable where appropriate: JDK-6911261: Project Coin: Retrofit Automatic Resource Management (ARM) support onto platform APIs https://bugs.openjdk.java.net/browse/JDK-6911261 JDK-6963723: Project Coin: Retrofit more JDK classes for ARM https://bugs.openjdk.java.net/browse/JDK-6963723 If you'll excuse the bad import into another blogging system, the process of finding the candidate types with an annotation processor and doing the retrofitting was discussed in a blog entry from that time: https://blogs.oracle.com/darcy/project-coin:-bringing-it-to-a-closeable More recently fixed in JDK 14, JDK-8203036: com.sun.net.httpserver.HttpExchange should implement AutoCloseable https://bugs.openjdk.java.net/browse/JDK-8203036 and as of August 2019, closed as will not fix given lack of interest: JDK-7057061: Support autocloseable for javax.naming.Context https://bugs.openjdk.java.net/browse/JDK-7057061 Thanks, -Joe On 4/15/2020 5:28 PM, Johannes Kuhn wrote:
After failing to wrap a XMLStreamReader in a try-with-resources after discovering it's close() method, I thought about checking what other classes have a public void close() method in the JDK but don't implement AutoCloseable.
So I wrote a small program that enumerates all classes present in in the jrt image with a public void close() method that does not directly or indirectly implement AutoCloseable: https://gist.github.com/DasBrain/8d50e02e35654870e2c2c00bf3f79954 (Output & Code, Code requires JDK 14 with preview features and ASM)
As I am not an expert in any of those areas where classes showed up, and I don't know if it was an oversight, or if there is a good reason for not implementing AutoCloseable. Implementing AutoCloseable is a double edged sword, a few IDEs warn about resource leaks based on that interface. And treat an AutoCloseable passed to methods who return an AutoCloseable as the return value wraping the parameter.
I did look at a few classes: * javax.xml.stream.XMLStreamReader:Javadoc: "Frees any resources associated with this Reader. This method does not close the underlying input source." Better not implement it? * javax.naming.Context: Javadoc: "This method releases this context's resources immediately, instead of waiting for them to be released automatically by the garbage collector." Implement AutoCloseable? * java.util.logging.Handler: Lifecycle seems to be managed by a LogManager. Better not implement AutoCloseable? * jdk.internal.jrtfs.ExplodedImage: class is package private, overrides close() from SystemImage while increasing visibility to public.
I did not restrict my enumeration to public and exported types - it was easier not to do that and I'm lazy.
This touches many unrelated areas, but David Holmes suggested that I take it to core-libs-dev. I know, I just did some static code analysis, but it seems that there might be some low hanging fruits. And for public classes I wish to have at least some rationale why a class with a public void close() method doesn't implement AutoCloseable.
Thanks, Johannes
Hi Joe, thanks for all the info. As I'm not yet familiar with the way how OpenJDK is developed, mind if I ask what steps would have to be taken to add Closeable to javax.naming.Context? The first thing would be to create a patch, test it, run the tests. Then the change has to be proposed, and reviewed. You can argue that this might waste the time of the reviewers with such a small patch. Is there something else that would need to be done? I have read http://openjdk.java.net/contribute/ and signed the OCA, but on the other hand, it's such a small change that most already know how the patch will look. I am not even sure if my time would be spend wisely. The motivation would be to do it to learn how to contribute. Anyway, thank you for your time and consideration. This just some stuff I found, then wrote some code, got some results and did not know what I can or should do with that. Thanks, Johannes On 16-Apr-20 3:15, Joe Darcy wrote:
Hi Johannes,
FYI, back during Project Coin during JDK 7, there were various systematic efforts to review types with a "close" method in the JDK and retrofit AutoCloseable where appropriate:
JDK-6911261: Project Coin: Retrofit Automatic Resource Management (ARM) support onto platform APIs https://bugs.openjdk.java.net/browse/JDK-6911261
JDK-6963723: Project Coin: Retrofit more JDK classes for ARM https://bugs.openjdk.java.net/browse/JDK-6963723
If you'll excuse the bad import into another blogging system, the process of finding the candidate types with an annotation processor and doing the retrofitting was discussed in a blog entry from that time:
https://blogs.oracle.com/darcy/project-coin:-bringing-it-to-a-closeable
More recently fixed in JDK 14,
JDK-8203036: com.sun.net.httpserver.HttpExchange should implement AutoCloseable https://bugs.openjdk.java.net/browse/JDK-8203036
and as of August 2019, closed as will not fix given lack of interest:
JDK-7057061: Support autocloseable for javax.naming.Context https://bugs.openjdk.java.net/browse/JDK-7057061
Thanks,
-Joe
On 4/15/2020 5:28 PM, Johannes Kuhn wrote:
After failing to wrap a XMLStreamReader in a try-with-resources after discovering it's close() method, I thought about checking what other classes have a public void close() method in the JDK but don't implement AutoCloseable.
So I wrote a small program that enumerates all classes present in in the jrt image with a public void close() method that does not directly or indirectly implement AutoCloseable: https://gist.github.com/DasBrain/8d50e02e35654870e2c2c00bf3f79954 (Output & Code, Code requires JDK 14 with preview features and ASM)
As I am not an expert in any of those areas where classes showed up, and I don't know if it was an oversight, or if there is a good reason for not implementing AutoCloseable. Implementing AutoCloseable is a double edged sword, a few IDEs warn about resource leaks based on that interface. And treat an AutoCloseable passed to methods who return an AutoCloseable as the return value wraping the parameter.
I did look at a few classes: * javax.xml.stream.XMLStreamReader:Javadoc: "Frees any resources associated with this Reader. This method does not close the underlying input source." Better not implement it? * javax.naming.Context: Javadoc: "This method releases this context's resources immediately, instead of waiting for them to be released automatically by the garbage collector." Implement AutoCloseable? * java.util.logging.Handler: Lifecycle seems to be managed by a LogManager. Better not implement AutoCloseable? * jdk.internal.jrtfs.ExplodedImage: class is package private, overrides close() from SystemImage while increasing visibility to public.
I did not restrict my enumeration to public and exported types - it was easier not to do that and I'm lazy.
This touches many unrelated areas, but David Holmes suggested that I take it to core-libs-dev. I know, I just did some static code analysis, but it seems that there might be some low hanging fruits. And for public classes I wish to have at least some rationale why a class with a public void close() method doesn't implement AutoCloseable.
Thanks, Johannes
On 16/04/2020 01:28, Johannes Kuhn wrote:
:
I did not restrict my enumeration to public and exported types - it was easier not to do that and I'm lazy. Most of the candidates that you've identified are JDK internal classes and there are several non-public classes in exported packages. There are also a few cases where the sub-classes of the likely candidate are showing up too (e.g. all the public sub-classes of java.util.logging.Handler). I skimmed through your list and filtered it down to the public classes in exported packages to following:
com/sun/jdi/connect/spi/Connection java/awt/SplashScreen java/util/logging/Handler javax/naming/Context javax/naming/NamingEnumeration javax/naming/ldap/StartTlsResponse javax/smartcardio/CardChannel javax/sql/PooledConnection javax/swing/ProgressMonitor javax/xml/stream/XMLEventReader javax/xml/stream/XMLEventWriter javax/xml/stream/XMLStreamReader javax/xml/stream/XMLStreamWriter As Joe points out, some of these may have been looked at already. I was surprised to see the java.xml.stream reader/writers so it might be worth digging into those to see why they were not retrofitted. -Alan.
updated the code, similar output. Disregard org/w3c/dom/html/HTMLDocument https://gist.github.com/DasBrain/df25a69acf564dcb5f2241cc065be2ed About java.xml.stream: A XMLStreamReader is for example constructed with: BufferedReader br = ...; XMLStreamReader xsr = XMLInputFactory.newDefaultFactory().createXMLEventReader(br); For my IDE this would look like XMLStreamReader wraps the BufferedReader, so it won't issue a warning if I don't close br. But the Javadoc for XMLStreamReader.close() states:
Frees any resources associated with this Reader. This method does not close the underlying input source.
If those classes would implement AutoCloseable, people and IDEs might wrongly assume that it would close the BufferedReader. Which would result in code like this: try (var xsr = XMLInputFactory.newDefaultFactory().createXMLEventReader(Files.newBufferedReader(path, StandardCharsets.UTF_8))) { ... } And have a resource leak. Incidentally, that is the similar to the code I tried to write when I saw that XMLStreamReader has a public void close() method. So in my opinion, to let XMLStreamReader implement AutoCloseable, the specification of the close() method should be changed. The same applies for the other XML Stream/Event Reader/Writer. Thanks, Johannes On 16-Apr-20 9:28, Alan Bateman wrote:
On 16/04/2020 01:28, Johannes Kuhn wrote:
:
I did not restrict my enumeration to public and exported types - it was easier not to do that and I'm lazy. Most of the candidates that you've identified are JDK internal classes and there are several non-public classes in exported packages. There are also a few cases where the sub-classes of the likely candidate are showing up too (e.g. all the public sub-classes of java.util.logging.Handler). I skimmed through your list and filtered it down to the public classes in exported packages to following:
com/sun/jdi/connect/spi/Connection java/awt/SplashScreen java/util/logging/Handler javax/naming/Context javax/naming/NamingEnumeration javax/naming/ldap/StartTlsResponse javax/smartcardio/CardChannel javax/sql/PooledConnection javax/swing/ProgressMonitor javax/xml/stream/XMLEventReader javax/xml/stream/XMLEventWriter javax/xml/stream/XMLStreamReader javax/xml/stream/XMLStreamWriter
As Joe points out, some of these may have been looked at already. I was surprised to see the java.xml.stream reader/writers so it might be worth digging into those to see why they were not retrofitted.
-Alan.
Hi Johannes, On 4/16/20 3:09 AM, Johannes Kuhn wrote:
About java.xml.stream: A XMLStreamReader is for example constructed with:
BufferedReader br = ...; XMLStreamReader xsr = XMLInputFactory.newDefaultFactory().createXMLEventReader(br);
For my IDE this would look like XMLStreamReader wraps the BufferedReader, so it won't issue a warning if I don't close br. But the Javadoc for XMLStreamReader.close() states:
Frees any resources associated with this Reader. This method does not close the underlying input source.
The typical try-with-resources idiom for this looks like: try (var br = new BufferedReader(...); var xsr = XMLInputFactory.newDefaultFactory().createXMLEventReader(br)) { ... } This should work even though XMLEventReader.close() is specified not to close the underlying Reader. It will also work to close the BufferedReader if the creation of XMLEventReader throws an exception. (The usual objection to this construct is if closing the wrapper closes the underlying reader, the t-w-r will close it again. This isn't a problem for BufferedReader and most JDK I/O classes, as close() is idempotent for them.)
If those classes would implement AutoCloseable, people and IDEs might wrongly assume that it would close the BufferedReader. Which would result in code like this:
try (var xsr = XMLInputFactory.newDefaultFactory().createXMLEventReader(Files.newBufferedReader(path, StandardCharsets.UTF_8))) { ... }
And have a resource leak. Incidentally, that is the similar to the code I tried to write when I saw that XMLStreamReader has a public void close() method.
I'm not sure what to make of what an IDE tells you. If it's making unwarranted assumptions about whether XMLEventReader (or some other wrapper) does or does not close the wrapped object, then it's a bug. Note that the nested construction example shown here try (var xsr = ....createXMLEventReader(Files.newBufferedReader(...))) *will* leak the BufferedReader if createXMLEventReader() throws an exception, since nobody has kept the reference to the BufferedReader. So in all cases, I recommend that people use the multiple variable declaration form of t-w-r.
So in my opinion, to let XMLStreamReader implement AutoCloseable, the specification of the close() method should be changed.
This would be an incompatible change. There might be code that relies on XMLStreamReader not to close the underlying reader, and reuses the reader for something.
The same applies for the other XML Stream/Event Reader/Writer.
On the face of it, making these AutoCloseable seems reasonable to me. However, I'm somewhat distant from this area, so I don't know if that makes sense for these particular XML interfaces. s'marks
Thanks, Johannes
On 16-Apr-20 9:28, Alan Bateman wrote:
On 16/04/2020 01:28, Johannes Kuhn wrote:
:
I did not restrict my enumeration to public and exported types - it was easier not to do that and I'm lazy. Most of the candidates that you've identified are JDK internal classes and there are several non-public classes in exported packages. There are also a few cases where the sub-classes of the likely candidate are showing up too (e.g. all the public sub-classes of java.util.logging.Handler). I skimmed through your list and filtered it down to the public classes in exported packages to following:
com/sun/jdi/connect/spi/Connection java/awt/SplashScreen java/util/logging/Handler javax/naming/Context javax/naming/NamingEnumeration javax/naming/ldap/StartTlsResponse javax/smartcardio/CardChannel javax/sql/PooledConnection javax/swing/ProgressMonitor javax/xml/stream/XMLEventReader javax/xml/stream/XMLEventWriter javax/xml/stream/XMLStreamReader javax/xml/stream/XMLStreamWriter
As Joe points out, some of these may have been looked at already. I was surprised to see the java.xml.stream reader/writers so it might be worth digging into those to see why they were not retrofitted.
-Alan.
On 4/21/20 10:06 PM, Stuart Marks wrote:
(The usual objection to this construct is if closing the wrapper closes the underlying reader, the t-w-r will close it again. This isn't a problem for BufferedReader and most JDK I/O classes, as close() is idempotent for them.)
So here's a Java design question (or two) for you: 1. Should close() always be idempotent, where practical? I would have thought so, but perhaps there are downsides. 2. Should classes which implement close() with the standard meaning be AutoCloseable? -- Andrew Haley (he/him) Java Platform Lead Engineer Red Hat UK Ltd. <https://www.redhat.com> https://keybase.io/andrewhaley EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
On 22/04/2020 13:50, Andrew Haley wrote:
: 1. Should close() always be idempotent, where practical? I would have thought so, but perhaps there are downsides.
2. Should classes which implement close() with the standard meaning be AutoCloseable?
I'm sure Joe Darcy can say more on this but I remember there was a lot of effort put into this topic when AutoCloseable was added in Java 7 (and Project Coin). Closeable close is idempotent but AutoCloseable close could not require it. AutoCloseable's API docs recommend it of course. There was effort in Java 7 and beyond to retrofit existing classes that defined a close method to be Closeable or AutoCloseable. There are only a handful of exported APIs remaining that have a close method that don't extend or implement AutoCloseable. I don't know the history of the XML stream interface to know why they close to define them not to close the underlying stream but I doubt these could be changed now. -Alan.
Hello, On 4/22/2020 6:12 AM, Alan Bateman wrote:
On 22/04/2020 13:50, Andrew Haley wrote:
: 1. Should close() always be idempotent, where practical? I would have thought so, but perhaps there are downsides.
2. Should classes which implement close() with the standard meaning be AutoCloseable?
I'm sure Joe Darcy can say more on this but I remember there was a lot of effort put into this topic when AutoCloseable was added in Java 7 (and Project Coin). Closeable close is idempotent but AutoCloseable close could not require it. AutoCloseable's API docs recommend it of course. There was effort in Java 7 and beyond to retrofit existing classes that defined a close method to be Closeable or AutoCloseable. There are only a handful of exported APIs remaining that have a close method that don't extend or implement AutoCloseable. I don't know the history of the XML stream interface to know why they close to define them not to close the underlying stream but I doubt these could be changed now.
Yes, the JSR 334 EG had some discussions about both "SilentCloseable" (no exceptions) and "IdempotentCloseable" as possible library additions back in the JDK 7 time frame. These were not judged to have sufficient marginal utility over Closeable and AutoCloseable to include in the platform. It was impractical to require idempotent close methods in call cases, but they are recommended. Generally, a type with a void close method should implement AutoClosable. Most of the candidate types in the JDK were updated for JDK 7; a few stragglers were updated since then, but there are a few remaining cases that could be updated as discussed earlier in this thread. HTH, -Joe
On 4/22/20 1:42 PM, Joe Darcy wrote:
On 4/22/2020 6:12 AM, Alan Bateman wrote:
On 22/04/2020 13:50, Andrew Haley wrote:
: 1. Should close() always be idempotent, where practical? I would have thought so, but perhaps there are downsides.
2. Should classes which implement close() with the standard meaning be AutoCloseable?
I'm sure Joe Darcy can say more on this but I remember there was a lot of effort put into this topic when AutoCloseable was added in Java 7 (and Project Coin). Closeable close is idempotent but AutoCloseable close could not require it. AutoCloseable's API docs recommend it of course. There was effort in Java 7 and beyond to retrofit existing classes that defined a close method to be Closeable or AutoCloseable. There are only a handful of exported APIs remaining that have a close method that don't extend or implement AutoCloseable. I don't know the history of the XML stream interface to know why they close to define them not to close the underlying stream but I doubt these could be changed now.
Yes, the JSR 334 EG had some discussions about both "SilentCloseable" (no exceptions) and "IdempotentCloseable" as possible library additions back in the JDK 7 time frame. These were not judged to have sufficient marginal utility over Closeable and AutoCloseable to include in the platform.
It was impractical to require idempotent close methods in call cases, but they are recommended. Generally, a type with a void close method should implement AutoClosable. Most of the candidate types in the JDK were updated for JDK 7; a few stragglers were updated since then, but there are a few remaining cases that could be updated as discussed earlier in this thread.
Regarding idempotentcy, I remember some of those discussions. To the extent possible, JDK implementations all have idempotent close() methods. However, some classes that seemed useful to be AutoCloseable were extensible outside the JDK, and we couldn't guarantee the idempotency of those close() implementations. It seemed reasonable to make the JDK classes AutoCloseable but for AC not to require idempotency. To answer Andrew's second question, I think whether something ought to implement AC this depends on the "standard meaning" of close(), but the answer is likely yes. I think the "standard meaning" includes some ideas such as the possibility of the object referencing some resource external to the JVM, not under (direct) control of the garbage collector, for which it would be considered a resource leak for close() not to be called, and for which prompt release of that resource is considered important. There's another small wrinkle with AutoCloseable which is that its definition changed somewhat in Java 8. Prior to Java 8 there was a sense that any AC instance ought to be used within a try-with-resources statement, and not doing so merited a warning. In Java 8 this was relaxed somewhat, in that T-W-R should be used only for AC instances that are known to contain external resources. The driver here was Stream, which implements AC. A Stream might represent a resource -- see Files.lines() for example -- in which case T-W-R should be used. However, many streams represent only in-memory values, so in those cases T-W-R is superfluous. Finally, several years back there was a bit of discussion on core-libs-dev on the issue of whether the XML streams should implement AutoCloseable; see http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-May/017343.html At the time the main issue seemed to be a complication with changing the specifications, because these APIs were also under the control of an independent (non Java SE platform) JSR. In fact that might have been the original reason for these APIs not to have been retrofitted in Java 7. Given the passage of time, perhaps this is no longer an issue. s'marks
Hi Johannes, On 16/04/2020 08:28, Alan Bateman wrote:
java/util/logging/Handler
That's already logged as https://bugs.openjdk.java.net/browse/JDK-8025709 (assigned to me) I have never got to the point to start working on it because it seems to be a very niche case: Handler are usually attached to loggers, so unless you're writing a test or using the API in ways that it was not designed for (though they might still be legitimate), then it's very unlikely that you would use a Handler in a try with resource statement. The java.logging module is mainly in maintenance mode these days and any changes we do there have a tendency to ripple out of control and come back to bite us - so we usually tend to exercise caution. best regards, -- daniel
As Joe points out, some of these may have been looked at already. I was surprised to see the java.xml.stream reader/writers so it might be worth digging into those to see why they were not retrofitted.
participants (6)
-
Alan Bateman
-
Andrew Haley
-
Daniel Fuchs
-
Joe Darcy
-
Johannes Kuhn
-
Stuart Marks