Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8343768

Inflater and Deflater should implement AutoCloseable

XMLWordPrintable

    • Icon: CSR CSR
    • Resolution: Unresolved
    • Icon: P4 P4
    • 25
    • core-libs
    • None
    • source, binary, behavioral
    • low
    • Hide
      The change introduces a new `close()` method marked `final` on the `Inflater` and `Deflater` classes. Both these classes are allowed to have subclasses and thus may have subclasses belonging to libraries or applications. It's possible that some of these existing subclasses may already have a `close()` method and thus this change can cause source and binary compatibility issues for those classes. We have run a corpus analysis against vast amount of classes and among those there have been some classes which extend Inflater or Deflater, but we haven't found a close() method defined in any of those subclasses. So the chances of the compatibility impact are low.

      The other change we do here is to throw IllegalStateException instead of the (unspecified) NullPointerException when certain methods on Inflater and Deflater are inovoked on a closed instance. The chances of this impacting existing applications can be considered very minimal because it's highly unlikely that there would be a realistic situation where an application relies on the (unspecified) expectation that a NullPointerException will get thrown from closed Inflater or Deflater instance.
      Show
      The change introduces a new `close()` method marked `final` on the `Inflater` and `Deflater` classes. Both these classes are allowed to have subclasses and thus may have subclasses belonging to libraries or applications. It's possible that some of these existing subclasses may already have a `close()` method and thus this change can cause source and binary compatibility issues for those classes. We have run a corpus analysis against vast amount of classes and among those there have been some classes which extend Inflater or Deflater, but we haven't found a close() method defined in any of those subclasses. So the chances of the compatibility impact are low. The other change we do here is to throw IllegalStateException instead of the (unspecified) NullPointerException when certain methods on Inflater and Deflater are inovoked on a closed instance. The chances of this impacting existing applications can be considered very minimal because it's highly unlikely that there would be a realistic situation where an application relies on the (unspecified) expectation that a NullPointerException will get thrown from closed Inflater or Deflater instance.
    • Java API
    • SE

      Summary

      java.util.zip.Deflater and java.util.zip.Inflater classes will be enhanced to implement java.lang.AutoCloseable.

      Problem

      The Deflater and Inflater classes have APIs which allow applications to compress and decompress data, respectively. The implementation in these classes uses the ZLIB native library. Compression and decompression operations using this library require the use of native resources whose lifetime is controlled by the corresponding Deflater/Inflater instance. The application must release these resources by calling the end() method.

      Conventionally, Java APIs use the close() method for releasing resources. The use of the end() method for this purpose is unusual and is easy for developers to miss. The Deflater and Inflater classes have no close() methods. They thus do not implement the AutoCloseable interface, and in turn they cannot be used with the try-with-resources statement. This situation is inconvenient and error-prone.

      Making Inflater and Deflater implement the AutoCloseable will make it more prominent that the instances of these classes hold on to resources that are expected to be cleaned up when the instances are no longer need by the application. Furthermore, various static analysis tools, including Integrated Development Environments (IDEs) have the ability to warn application developers if instances of AutoCloseable aren't closed. Thus, having Inflater and Deflater implement AutoCloseable will increase the chances of the application developer closing these instances at the right places.

      Solution

      The Inflater and Deflater classes will be enhanced to implement the AutoCloseable interface. The newly introduced close() method will be specified to invoke the current existing end() method that is already specified to clean up resources. The close() method will be allowed to be invoked more than once and each time it will invoke the end() method. The specification of the end() method will be updated to state that it allows multiple invocations to it but the subsequent invocations after the first one will act as no-op.

      The newly introduced close() method will also be marked as final on both Inflater and Deflater classes to make it clear to subclasses that any resources they allocate must be freed by overriding the end() method.

      While working on these changes, it was noticed that the current implementation of both the Inflater and Deflater throw a (unspecified) NullPointerException if certain methods are invoked on the instances after the Inflater/Deflater has been closed. These methods will be updated to specify that they will throw an java.lang.IllegalStateException if they are invoked on closed instances. The implementation of these methods will thus be updated to throw an IllegalStateException instead of a NullPointerException, for such cases.

      Specification

        diff --git a/src/java.base/share/classes/java/util/zip/Deflater.java b/src/java.base/share/classes/java/util/zip/Deflater.java
      index a23f8395c2178..74bea335083bb 100644
      --- a/src/java.base/share/classes/java/util/zip/Deflater.java
      +++ b/src/java.base/share/classes/java/util/zip/Deflater.java
      @@ -49,29 +49,31 @@
        * thrown.
        * <p>
        * This class deflates sequences of bytes into ZLIB compressed data format.
      - * The input byte sequence is provided in either byte array or byte buffer,
      + * The input byte sequence is provided in either byte array or {@link ByteBuffer},
        * via one of the {@code setInput()} methods. The output byte sequence is
      - * written to the output byte array or byte buffer passed to the
      + * written to the output byte array or {@code ByteBuffer} passed to the
        * {@code deflate()} methods.
        * <p>
      - * The following code fragment demonstrates a trivial compression
      - * and decompression of a string using {@code Deflater} and
      - * {@code Inflater}.
      - * {@snippet id="compdecomp" lang="java" class="Snippets" region="DeflaterInflaterExample"}
      + * To release resources used by the {@code Deflater}, applications must call the
      + * {@link #end()} method. After {@code end()} has been called, subsequent calls
      + * to several methods of the {@code Deflater} will throw an {@link IllegalStateException}.
        *
        * @apiNote
      - * To release resources used by this {@code Deflater}, the {@link #end()} method
      - * should be called explicitly. Subclasses are responsible for the cleanup of resources
      - * acquired by the subclass. Subclasses that override {@link #finalize()} in order
      - * to perform cleanup should be modified to use alternative cleanup mechanisms such
      - * as {@link java.lang.ref.Cleaner} and remove the overriding {@code finalize} method.
      + * This class implements {@link AutoCloseable} to facilitate its usage with
      + * {@code try}-with-resources statement. The {@linkplain Deflater#close() close() method} simply
      + * calls {@code end()}.
      + *
      + * <p>
      + * The following code fragment demonstrates a trivial compression
      + * and decompression of a string using {@code Deflater} and {@code Inflater}.
      + * {@snippet id="compdecomp" lang="java" class="Snippets" region="DeflaterInflaterExample"}
        *
        * @see         Inflater
        * @author      David Connelly
        * @since 1.1
        */
      
      -public class Deflater {
      +public class Deflater implements AutoCloseable {
      
           private final DeflaterZStreamRef zsRef;
           private ByteBuffer input = ZipUtils.defaultBuf;
      @@ -269,6 +271,7 @@ public void setInput(ByteBuffer input) {
            * @param dictionary the dictionary data bytes
            * @param off the start offset of the data
            * @param len the length of the data
      +     * @throws IllegalStateException if the Deflater is closed
            * @see Inflater#inflate
            * @see Inflater#getAdler()
            */
      @@ -287,6 +290,7 @@ public void setDictionary(byte[] dictionary, int off, int len) {
            * in order to get the Adler-32 value of the dictionary required for
            * decompression.
            * @param dictionary the dictionary data bytes
      +     * @throws IllegalStateException if the Deflater is closed
            * @see Inflater#inflate
            * @see Inflater#getAdler()
            */
      @@ -305,6 +309,7 @@ public void setDictionary(byte[] dictionary) {
            * return, its position will equal its limit.
            *
            * @param dictionary the dictionary data bytes
      +     * @throws IllegalStateException if the Deflater is closed
            * @see Inflater#inflate
            * @see Inflater#getAdler()
            *
      @@ -435,6 +440,7 @@ public boolean finished() {
            * @param output the buffer for the compressed data
            * @param off the start offset of the data
            * @param len the maximum number of bytes of compressed data
      +     * @throws IllegalStateException if the Deflater is closed
            * @return the actual number of bytes of compressed data written to the
            *         output buffer
            */
      @@ -454,6 +460,7 @@ public int deflate(byte[] output, int off, int len) {
            * {@code deflater.deflate(b, 0, b.length, Deflater.NO_FLUSH)}.
            *
            * @param output the buffer for the compressed data
      +     * @throws IllegalStateException if the Deflater is closed
            * @return the actual number of bytes of compressed data written to the
            *         output buffer
            */
      @@ -476,6 +483,7 @@ public int deflate(byte[] output) {
            * @return the actual number of bytes of compressed data written to the
            *         output buffer
            * @throws ReadOnlyBufferException if the given output buffer is read-only
      +     * @throws IllegalStateException if the Deflater is closed
            * @since 11
            */
           public int deflate(ByteBuffer output) {
      @@ -531,6 +539,7 @@ public int deflate(ByteBuffer output) {
            *         the output buffer
            *
            * @throws IllegalArgumentException if the flush mode is invalid
      +     * @throws IllegalStateException if the Deflater is closed
            * @since 1.7
            */
           public int deflate(byte[] output, int off, int len, int flush) {
      @@ -657,6 +666,7 @@ public int deflate(byte[] output, int off, int len, int flush) {
            *
            * @throws IllegalArgumentException if the flush mode is invalid
            * @throws ReadOnlyBufferException if the given output buffer is read-only
      +     * @throws IllegalStateException if the Deflater is closed
            * @since 11
            */
           public int deflate(ByteBuffer output, int flush) {
      @@ -783,6 +793,7 @@ public int deflate(ByteBuffer output, int flush) {
      
           /**
            * {@return the ADLER-32 value of the uncompressed data}
      +     * @throws IllegalStateException if the Deflater is closed
            */
           public int getAdler() {
               synchronized (zsRef) {
      @@ -799,6 +810,8 @@ public int getAdler() {
            * and therefore cannot return the correct value when it is greater
            * than {@link Integer#MAX_VALUE}.
            *
      +     * @throws IllegalStateException if the Deflater is closed
      +     *
            * @deprecated Use {@link #getBytesRead()} instead
            *
            * @return the total number of uncompressed bytes input so far
      @@ -812,6 +825,7 @@ public int getTotalIn() {
            * Returns the total number of uncompressed bytes input so far.
            *
            * @return the total (non-negative) number of uncompressed bytes input so far
      +     * @throws IllegalStateException if the Deflater is closed
            * @since 1.5
            */
           public long getBytesRead() {
      @@ -829,6 +843,8 @@ public long getBytesRead() {
            * and therefore cannot return the correct value when it is greater
            * than {@link Integer#MAX_VALUE}.
            *
      +     * @throws IllegalStateException if the Deflater is closed
      +     *
            * @deprecated Use {@link #getBytesWritten()} instead
            *
            * @return the total number of compressed bytes output so far
      @@ -842,6 +858,7 @@ public int getTotalOut() {
            * Returns the total number of compressed bytes output so far.
            *
            * @return the total (non-negative) number of compressed bytes output so far
      +     * @throws IllegalStateException if the Deflater is closed
            * @since 1.5
            */
           public long getBytesWritten() {
      @@ -854,6 +871,7 @@ public long getBytesWritten() {
           /**
            * Resets deflater so that a new set of input data can be processed.
            * Keeps current compression level and strategy settings.
      +     * @throws IllegalStateException if the Deflater is closed
            */
           public void reset() {
               synchronized (zsRef) {
      @@ -868,23 +886,44 @@ public void reset() {
           }
      
           /**
      -     * Closes the compressor and discards any unprocessed input.
      +     * Closes and releases the resources held by this {@code Deflater}
      +     * and discards any unprocessed input.
      +     * <p>
      +     * If this method is invoked multiple times, the second and subsequent calls do nothing.
            *
      -     * This method should be called when the compressor is no longer
      -     * being used. Once this method is called, the behavior of the
      -     * Deflater object is undefined.
      +     * @see #close()
            */
           public void end() {
               synchronized (zsRef) {
      +            // check if already closed
      +            if (zsRef.address() == 0) {
      +                return;
      +            }
                   zsRef.clean();
                   input = ZipUtils.defaultBuf;
      +            inputArray = null;
               }
           }
      
      +    /**
      +     * Closes and releases the resources held by this {@code Deflater}
      +     * and discards any unprocessed input.
      +     *
      +     * @implSpec This method calls the {@link #end()} method.
      +     * @since 24
      +     */
      +    @Override
      +    public final void close() {
      +        end();
      +    }
      +
           private void ensureOpen() {
               assert Thread.holdsLock(zsRef);
      -        if (zsRef.address() == 0)
      -            throw new NullPointerException("Deflater has been closed");
      +        if (zsRef.address() == 0) {
      +            throw new IllegalStateException("Deflater has been closed");
      +        }
           }
      
      diff --git a/src/java.base/share/classes/java/util/zip/Inflater.java b/src/java.base/share/classes/java/util/zip/Inflater.java
      index 109208bd9ad8c..18363a2b79b6e 100644
      --- a/src/java.base/share/classes/java/util/zip/Inflater.java
      +++ b/src/java.base/share/classes/java/util/zip/Inflater.java
      @@ -49,21 +49,23 @@
        * thrown.
        * <p>
        * This class inflates sequences of ZLIB compressed bytes. The input byte
      - * sequence is provided in either byte array or byte buffer, via one of the
      + * sequence is provided in either byte array or {@link ByteBuffer}, via one of the
        * {@code setInput()} methods. The output byte sequence is written to the
      - * output byte array or byte buffer passed to the {@code inflate()} methods.
      + * output byte array or {@code ByteBuffer} passed to the {@code inflate()} methods.
        * <p>
      - * The following code fragment demonstrates a trivial compression
      - * and decompression of a string using {@code Deflater} and
      - * {@code Inflater}.
      - * {@snippet id="compdecomp" lang="java" class="Snippets" region="DeflaterInflaterExample"}
      + * To release resources used by the {@code Inflater}, applications must call the
      + * {@link #end()} method. After {@code end()} has been called, subsequent calls
      + * to several methods of the {@code Inflater} will throw an {@link IllegalStateException}.
        *
        * @apiNote
      - * To release resources used by this {@code Inflater}, the {@link #end()} method
      - * should be called explicitly. Subclasses are responsible for the cleanup of resources
      - * acquired by the subclass. Subclasses that override {@link #finalize()} in order
      - * to perform cleanup should be modified to use alternative cleanup mechanisms such
      - * as {@link java.lang.ref.Cleaner} and remove the overriding {@code finalize} method.
      + * This class implements {@link AutoCloseable} to facilitate its usage with
      + * {@code try}-with-resources statement. The {@linkplain Inflater#close() close() method} simply
      + * calls {@code end()}.
      + *
      + * <p>
      + * The following code fragment demonstrates a trivial compression
      + * and decompression of a string using {@code Deflater} and {@code Inflater}.
      + * {@snippet id="compdecomp" lang="java" class="Snippets" region="DeflaterInflaterExample"}
        *
        * @see         Deflater
        * @author      David Connelly
      @@ -71,7 +73,7 @@
        *
        */
      
      -public class Inflater {
      +public class Inflater implements AutoCloseable {
      
           private final InflaterZStreamRef zsRef;
           private ByteBuffer input = ZipUtils.defaultBuf;
      @@ -192,6 +194,7 @@ public void setInput(ByteBuffer input) {
            * @param dictionary the dictionary data bytes
            * @param off the start offset of the data
            * @param len the length of the data
      +     * @throws IllegalStateException if the Inflater is closed
            * @see Inflater#needsDictionary
            * @see Inflater#getAdler
            */
      @@ -210,6 +213,7 @@ public void setDictionary(byte[] dictionary, int off, int len) {
            * indicating that a preset dictionary is required. The method getAdler()
            * can be used to get the Adler-32 value of the dictionary needed.
            * @param dictionary the dictionary data bytes
      +     * @throws IllegalStateException if the Inflater is closed
            * @see Inflater#needsDictionary
            * @see Inflater#getAdler
            */
      @@ -227,6 +231,7 @@ public void setDictionary(byte[] dictionary) {
            * return, its position will equal its limit.
            *
            * @param dictionary the dictionary data bytes
      +     * @throws IllegalStateException if the Inflater is closed
            * @see Inflater#needsDictionary
            * @see Inflater#getAdler
            * @since 11
      @@ -331,6 +336,7 @@ public boolean finished() {
            * @param len the maximum number of uncompressed bytes
            * @return the actual number of uncompressed bytes
            * @throws DataFormatException if the compressed data format is invalid
      +     * @throws IllegalStateException if the Inflater is closed
            * @see Inflater#needsInput
            * @see Inflater#needsDictionary
            */
      @@ -437,6 +443,7 @@ public int inflate(byte[] output, int off, int len)
            * @param output the buffer for the uncompressed data
            * @return the actual number of uncompressed bytes
            * @throws DataFormatException if the compressed data format is invalid
      +     * @throws IllegalStateException if the Inflater is closed
            * @see Inflater#needsInput
            * @see Inflater#needsDictionary
            */
      @@ -474,6 +481,7 @@ public int inflate(byte[] output) throws DataFormatException {
            * @return the actual number of uncompressed bytes
            * @throws DataFormatException if the compressed data format is invalid
            * @throws ReadOnlyBufferException if the given output buffer is read-only
      +     * @throws IllegalStateException if the Inflater is closed
            * @see Inflater#needsInput
            * @see Inflater#needsDictionary
            * @since 11
      @@ -600,6 +608,7 @@ public int inflate(ByteBuffer output) throws DataFormatException {
      
           /**
            * {@return the ADLER-32 value of the uncompressed data}
      +     * @throws IllegalStateException if the Inflater is closed
            */
           public int getAdler() {
               synchronized (zsRef) {
      @@ -616,6 +625,8 @@ public int getAdler() {
            * and therefore cannot return the correct value when it is greater
            * than {@link Integer#MAX_VALUE}.
            *
      +     * @throws IllegalStateException if the Inflater is closed
      +     *
            * @deprecated Use {@link #getBytesRead()} instead
            *
            * @return the total number of compressed bytes input so far
      @@ -629,6 +640,7 @@ public int getTotalIn() {
            * Returns the total number of compressed bytes input so far.
            *
            * @return the total (non-negative) number of compressed bytes input so far
      +     * @throws IllegalStateException if the Inflater is closed
            * @since 1.5
            */
           public long getBytesRead() {
      @@ -646,6 +658,8 @@ public long getBytesRead() {
            * and therefore cannot return the correct value when it is greater
            * than {@link Integer#MAX_VALUE}.
            *
      +     * @throws IllegalStateException if the Inflater is closed
      +     *
            * @deprecated Use {@link #getBytesWritten()} instead
            *
            * @return the total number of uncompressed bytes output so far
      @@ -659,6 +673,7 @@ public int getTotalOut() {
            * Returns the total number of uncompressed bytes output so far.
            *
            * @return the total (non-negative) number of uncompressed bytes output so far
      +     * @throws IllegalStateException if the Inflater is closed
            * @since 1.5
            */
           public long getBytesWritten() {
      @@ -670,6 +685,7 @@ public long getBytesWritten() {
      
           /**
            * Resets inflater so that a new set of input data can be processed.
      +     * @throws IllegalStateException if the Inflater is closed
            */
           public void reset() {
               synchronized (zsRef) {
      @@ -684,14 +700,21 @@ public void reset() {
           }
      
           /**
      -     * Closes the decompressor and discards any unprocessed input.
      +     * Closes and releases the resources held by this {@code Inflater}
      +     * and discards any unprocessed input.
      +     * <p>
      +     * If this method is invoked multiple times, the second and subsequent calls do nothing.
            *
      -     * This method should be called when the decompressor is no longer
      -     * being used. Once this method is called, the behavior of the
      -     * Inflater object is undefined.
      +     * @see #close()
            */
           public void end() {
               synchronized (zsRef) {
      +            // check if already closed
      +            if (zsRef.address() == 0) {
      +                return;
      +            }
                   zsRef.clean();
                   input = ZipUtils.defaultBuf;
                   inputArray = null;
      @@ -699,10 +722,23 @@ public void end() {
           }
      
      
      +    /**
      +     * Closes and releases the resources held by this {@code Inflater}
      +     * and discards any unprocessed input.
      +     *
      +     * @implSpec This method calls the {@link #end()} method.
      +     * @since 24
      +     */
      +    @Override
      +    public final void close() {
      +        end();
      +    }
      +

            jpai Jaikiran Pai
            smarks Stuart Marks
            Lance Andersen, Stuart Marks
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated: