RFR  JDK-8225763 Inflater and Deflater should implement AutoCloseable
jai.forums2013 at gmail.com
Wed Jul 10 05:47:17 UTC 2019
On 10/07/19 2:25 AM, Lance Andersen wrote:
> Hi Jaikiran,
> Again, thank you for your efforts here.
> Is there a CSR for this yet?
There isn't one yet. I will need help on this part, since I haven't
created a CSR before. Is this something that I can create (I'm just a
"author" in the JDK project)? If yes, is there some specific things I
need to do?
I'll reply to the rest of the review comments soon. Thank you very much
for doing this review and offering to sponsor.
> This will need to approved prior to moving forward with committing the
> I can sponsor once everything has been approved and finalized.
> @implSpec This method is a no-op if this compressor has already
> 886 * been previously closed,
> Please remove “already” in both the close() and end() methods. I
> believe the preference is the @implSpec and its relatives are on their
> own line as in https://openjdk.java.net/jeps/8068562 and was done for
> @apiNote earlier
> - Undefined behavior after close()/end()
> I am not convinced the new wording is an improvement and I know Stuart
> was not thrilled with the existing wording. However given the classes
> may be subclassed, I am not sure we can truly specify the behavior
> which could be why the original authors used that wording. Stuart
> Outside of the @ImplSpec, I am not sure the initial wording for end()
> and close() really need to differ:
> Closes the decompressor and discards any unprocessed input.
> Releases resources held by this decompressor and discards any
> unprocessed input.This method should be called when the
> decompressor is no longer needed
>> On Jul 9, 2019, at 9:18 AM, Jaikiran Pai <jai.forums2013 at gmail.com
>> <mailto:jai.forums2013 at gmail.com>> wrote:
>> 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. The latest version of the patch for
>> this change is now available at .
>> 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
>> - 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.
> We need to be specific in close() also for clarity
>> - 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.
> Please update @biug to include the bug number
>> - 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
> Overall they look OK. In your tests, you are testing the number of
> calls for the sub-classes not for Deflate/Inflate so I would either
> update your comments to clarify that or pull them into their own test
> Again, thank you for your efforts here.
> Andersen| Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering
> 1 Network Drive
> Burlington, MA 01803
> Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>
More information about the core-libs-dev