Inputs on patch for JDK-8225763? Inflater and Deflater should implement AutoCloseable

Jaikiran Pai jai.forums2013 at
Tue Jul 9 13:24:16 UTC 2019

Hello Lance,

On 08/07/19 11:16 PM, Lance Andersen wrote:
> Hi Jaikiran,
> Thank you for your efforts here.
>> On Jul 6, 2019, at 9:59 AM, Jaikiran Pai <jai.forums2013 at
>> <mailto:jai.forums2013 at>> wrote:
>>>  - Idempotency
>>> Yes, it looks to me like Inflater.end() and Deflater.end()
>>> implementations are idempotent. As you point out, overrides by
>>> subclasses might not be. We should be clear when we're talking about
>>> the specific implementatations of the end() methods in the Deflater
>>> and Inflater classes, or whether we're talking about the contracts
>>> defined by these method specifications on these classes and subtypes.
>>> The behavior of an implementation in a class can be specified with
>>> @implSpec without imposing this on the contract of subclasses. This is
>>> useful for subclasses that need to know the exact behavior of calling
>>> super.end(), and also for callers who know they have an instance of a
>>> particular class and not a subclass.
>>> The upshot is that while we might not have the end() method's contract
>>> specify idempotency, we can certainly say so in an @implSpec, if doing
>>> this will help. I'm not sure we'll actually need it in this case, but
>>> let's keep it in mind.
>> Thank you. Although not for end(), I have now used this @implSpec in the
>> first version of my patch[1].
> This should be done for end() as well to set expectations.  While
> close() is the preferred way to go moving forward, end() is not going
> anywhere and still needs to be a first class-citizen WRT documentation.

Done. I have added the @implSpec for end() too in the new updated
revision of the webrev. I have opened a separate RFR thread containing
that webrev.

>>> **
>>> If you think the issues are settled enough, maybe it's time for you to
>>> take a stab at a patch. 
>> The initial version of my patch is now ready at [1]. Here's a high level
>> summary of what's contained in this patch:
> Please start a review request thread so it is easier to follow.  Once
> you do that, I will reply to it..

Done - I have now created a new RFR thread for this here

> Also, reminder to update copyright dates.

The updated webrev in the RFR thread contains this update.

> For the tests, it would be best to add more comments.  Out of
> curiosity, was there a reason you chose not to use TestNG vs having
> just a main which invokes each test?

No specific reason. I'm still relatively new to the JDK codebase and
don't know when to use testng as against the regular main() driven
tests. I have now cleaned up the tests and converted them to testng in
the new webrev that I proposed.


More information about the core-libs-dev mailing list