Re: RFR [14] JDK-8225763 Inflater and Deflater should implement AutoCloseable

2019-07-13 Thread Jaikiran Pai
Hello Peter, On 13/07/19 2:50 PM, Peter Levart wrote: > Hi Jaikiran, > > On 7/12/19 9:07 AM, Jaikiran Pai wrote: >> The new updated webrev is at >> http://cr.openjdk.java.net/~jpai/webrev/8225763/4/webrev/ > > > I don't know if there are any custom subclasses of Inflater/Deflater > around and

Re: RFR [14] JDK-8225763 Inflater and Deflater should implement AutoCloseable

2019-07-13 Thread Peter Levart
Hi Jaikiran, On 7/12/19 9:07 AM, Jaikiran Pai wrote: The new updated webrev is at http://cr.openjdk.java.net/~jpai/webrev/8225763/4/webrev/ I don't know if there are any custom subclasses of Inflater/Deflater around and what they do, but hypothetically consider a subclass of Deflater

Re: RFR [14] JDK-8225763 Inflater and Deflater should implement AutoCloseable

2019-07-12 Thread Jaikiran Pai
Hello Lance, On 12/07/19 5:53 AM, Lance Andersen wrote: > Hi Jaikiran, >> I have now updated the javadoc of end() to be closer to the javadoc >> of close(). >> >> > > Its better but end() should include the wording  > > > This method should be called when the compressor is no longer needed.

Re: RFR [14] JDK-8225763 Inflater and Deflater should implement AutoCloseable

2019-07-11 Thread Lance Andersen
Hi Jaikiran, > On Jul 10, 2019, at 2:56 AM, Jaikiran Pai wrote: > > Hello Lance, > On 10/07/19 2:25 AM, Lance Andersen wrote: >> ——— >> @implSpec This method is a no-op if this compressor has already >> 886 * been previously closed, >> >> >> >> Please remove “already” in both the

Re: RFR [14] JDK-8225763 Inflater and Deflater should implement AutoCloseable

2019-07-10 Thread Jaikiran Pai
On 09/07/19 8:44 PM, David Lloyd wrote: > On Tue, Jul 9, 2019 at 8:19 AM Jaikiran Pai wrote: >> - In addition, I have sneaked in an unrelated change in this patch, into >> the Deflater.end() method: >> >> public void end() { >> synchronized (zsRef) { >> +// check if

Re: RFR [14] JDK-8225763 Inflater and Deflater should implement AutoCloseable

2019-07-10 Thread Jaikiran Pai
Hello Lance, On 10/07/19 2:25 AM, Lance Andersen wrote: > ——— > @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. Done. >  I believe the preference is the @implSpec and its

Re: RFR [14] JDK-8225763 Inflater and Deflater should implement AutoCloseable

2019-07-09 Thread Jaikiran Pai
Hello Lance, 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"

Re: RFR [14] JDK-8225763 Inflater and Deflater should implement AutoCloseable

2019-07-09 Thread Lance Andersen
Hi Jaikiran, Again, thank you for your efforts here. Is there a CSR for this yet? This will need to approved prior to moving forward with committing the feature. I can sponsor once everything has been approved and finalized. ——— @implSpec This method is a no-op if this compressor has

Re: RFR [14] JDK-8225763 Inflater and Deflater should implement AutoCloseable

2019-07-09 Thread David Lloyd
On Tue, Jul 9, 2019 at 8:19 AM Jaikiran Pai wrote: > - 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() ==

RFR [14] JDK-8225763 Inflater and Deflater should implement AutoCloseable

2019-07-09 Thread Jaikiran Pai
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