Can I please get a review and a sponsor for the patch that implements
the enhancement noted in https://bugs.openjdk.java.net/browse/JDK-8225763 ?
There has been a recent discussion about this change in the
core-libs-dev mailing list here[1]. The latest version of the patch for
this change is now available at [2].
Here's a summary of what's contained in this patch:
- The Inflater/Deflater class now implements AutoCloseable
- The class level javadoc of both these classes has been to updated to
use newer style of code, with try-with-resources, for the example
contained in that javadoc. The @apiNote in these class level javadoc has
also been updated to mention the use of close() method for releasing
resources.
- The javadoc of end() method in both these classes has been updated to
encourage the use of close() method instead of this one. It now also has
a @implSpec which states that it's a no-op if called more than once.
In addition, this javadoc has also been updated to replace the
"undefined behaviour" statement with a mention that the usage of the
Inflater/Deflater instance after a call to end() may throw an exception
in those subsequent usages. Please note that, there's no such explicit
mention in the javadoc of the (newly added) close() method because IMO,
it isn't needed for close() since I think it's kind of implied that a
closed resource can no longer be used for further operations.
- The new close() method has been added along with javadoc which uses an
@implSpec to state that it internally calls end()
- TotalInOut.java test has been updated to use the new
try-with-resources construct for the inflater and deflater it uses.
- A couple of new (testng) test classes have been added to test various
scenarios where close() and end() method get called. These test mostly
focus on ensuring that the close() and end(), either implicitly or
explicitly, get called the right number of times on the subclasses of
Inflater/Deflater.
- I have run the tests under test/jdk/java/util/zip/ by running:
jtreg -jdk:build/macosx-x86_64-server-release/images/jdk
test/jdk/java/util/zip/
and they have come back passing (except for failures/errors in
java/util/zip/ZipFile/TestZipFile.java, java/util/zip/3GBZipFiles.sh and
java/util/zip/DataDescriptorSignatureMissing.java - those issues though
are unrelated to this change and expected to fail, based on what I see
in their test report logs)
- In addition, I have sneaked in an unrelated change in this patch, into
the Deflater.end() method:
public void end() {
synchronized (zsRef) {
+ // check if already closed
+ if (zsRef.address() == 0) {
+ return;
+ }
zsRef.clean();
input = ZipUtils.defaultBuf;
+ inputArray = null;
+ }
Unlike in the Inflater.end(), the Deflater.end() didn't previously have
the "inputArray = null". I have included it in this patch, since it
looks right to clean up that array too. If this isn't the right
time/patch to do it, please do let me know and I'll take that up separately.
[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2019-June/061096.html
[2] http://cr.openjdk.java.net/~jpai/webrev/8225763/2/webrev/
-Jaikiran
