osdir.com

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [1/3] commons-compress git commit: COMPRESS-470 make sure all ScatterZipOutputStreams are closed


ensureStreamsAreClosed() seems like an over the top name. Why not simply
closeAll()? Or close().

Gary

On Sun, Nov 18, 2018, 09:02 <bodewig@xxxxxxxxxx wrote:

> Repository: commons-compress
> Updated Branches:
>   refs/heads/master 979260717 -> f132d6c50
>
>
> COMPRESS-470 make sure all ScatterZipOutputStreams are closed
>
>
> Project: http://git-wip-us.apache.org/repos/asf/commons-compress/repo
> Commit:
> http://git-wip-us.apache.org/repos/asf/commons-compress/commit/2f75fbb5
> Tree:
> http://git-wip-us.apache.org/repos/asf/commons-compress/tree/2f75fbb5
> Diff:
> http://git-wip-us.apache.org/repos/asf/commons-compress/diff/2f75fbb5
>
> Branch: refs/heads/master
> Commit: 2f75fbb5269bd5b2c7b799b7a4f93992fa1f9196
> Parents: 9792607
> Author: Stefan Bodewig <bodewig@xxxxxxxxxx>
> Authored: Sun Nov 18 16:59:31 2018 +0100
> Committer: Stefan Bodewig <bodewig@xxxxxxxxxx>
> Committed: Sun Nov 18 16:59:31 2018 +0100
>
> ----------------------------------------------------------------------
>  src/changes/changes.xml                             |  4 ++++
>  .../archivers/zip/ParallelScatterZipCreator.java    | 16 ++++++++++++++++
>  .../archivers/zip/ScatterZipOutputStream.java       |  5 +++++
>  3 files changed, 25 insertions(+)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/commons-compress/blob/2f75fbb5/src/changes/changes.xml
> ----------------------------------------------------------------------
> diff --git a/src/changes/changes.xml b/src/changes/changes.xml
> index 5b9619a..e16e763 100644
> --- a/src/changes/changes.xml
> +++ b/src/changes/changes.xml
> @@ -58,6 +58,10 @@ The <action> type attribute can be
> add,update,fix,remove.
>          TarArchiveInputStream has a new constructor-arg lenient that
>          can be used to accept certain broken archives.
>        </action>
> +      <action issue="COMPRESS-470" type="fix" date="2018-11-18">
> +        Fixed another potential resource leak in
> +        ParallelScatterZipCreator#writeTo.
> +      </action>
>      </release>
>      <release version="1.18" date="2018-08-16"
>               description="Release 1.18">
>
>
> http://git-wip-us.apache.org/repos/asf/commons-compress/blob/2f75fbb5/src/main/java/org/apache/commons/compress/archivers/zip/ParallelScatterZipCreator.java
> ----------------------------------------------------------------------
> diff --git
> a/src/main/java/org/apache/commons/compress/archivers/zip/ParallelScatterZipCreator.java
> b/src/main/java/org/apache/commons/compress/archivers/zip/ParallelScatterZipCreator.java
> index a381d0a..f2a1679 100644
> ---
> a/src/main/java/org/apache/commons/compress/archivers/zip/ParallelScatterZipCreator.java
> +++
> b/src/main/java/org/apache/commons/compress/archivers/zip/ParallelScatterZipCreator.java
> @@ -239,6 +239,7 @@ public class ParallelScatterZipCreator {
>      public void writeTo(final ZipArchiveOutputStream targetStream)
>              throws IOException, InterruptedException, ExecutionException {
>
> +        try {
>          // Make sure we catch any exceptions from parallel phase
>          try {
>              for (final Future<?> future : futures) {
> @@ -261,6 +262,9 @@ public class ParallelScatterZipCreator {
>          }
>
>          scatterDoneAt = System.currentTimeMillis();
> +        } finally {
> +            ensureStreamsAreClosed();
> +        }
>      }
>
>      /**
> @@ -271,5 +275,17 @@ public class ParallelScatterZipCreator {
>      public ScatterStatistics getStatisticsMessage() {
>          return new ScatterStatistics(compressionDoneAt - startedAt,
> scatterDoneAt - compressionDoneAt);
>      }
> +
> +    private void ensureStreamsAreClosed() {
> +        synchronized (streams) {
> +            for (final ScatterZipOutputStream scatterStream : streams) {
> +                try {
> +                    scatterStream.close();
> +                } catch (IOException ex) {
> +                    // no way to properly log this
> +                }
> +            }
> +        }
> +    }
>  }
>
>
>
> http://git-wip-us.apache.org/repos/asf/commons-compress/blob/2f75fbb5/src/main/java/org/apache/commons/compress/archivers/zip/ScatterZipOutputStream.java
> ----------------------------------------------------------------------
> diff --git
> a/src/main/java/org/apache/commons/compress/archivers/zip/ScatterZipOutputStream.java
> b/src/main/java/org/apache/commons/compress/archivers/zip/ScatterZipOutputStream.java
> index 7001c84..006c531 100644
> ---
> a/src/main/java/org/apache/commons/compress/archivers/zip/ScatterZipOutputStream.java
> +++
> b/src/main/java/org/apache/commons/compress/archivers/zip/ScatterZipOutputStream.java
> @@ -29,6 +29,7 @@ import java.io.IOException;
>  import java.io.InputStream;
>  import java.util.Queue;
>  import java.util.concurrent.ConcurrentLinkedQueue;
> +import java.util.concurrent.atomic.AtomicBoolean;
>  import java.util.zip.Deflater;
>
>  /**
> @@ -49,6 +50,7 @@ public class ScatterZipOutputStream implements Closeable
> {
>      private final Queue<CompressedEntry> items = new
> ConcurrentLinkedQueue<>();
>      private final ScatterGatherBackingStore backingStore;
>      private final StreamCompressor streamCompressor;
> +    private AtomicBoolean isClosed = new AtomicBoolean();
>
>      private static class CompressedEntry {
>          final ZipArchiveEntryRequest zipArchiveEntryRequest;
> @@ -124,6 +126,9 @@ public class ScatterZipOutputStream implements
> Closeable {
>       */
>      @Override
>      public void close() throws IOException {
> +        if (!isClosed.compareAndSet(false, true)) {
> +            return;
> +        }
>          try {
>              backingStore.close();
>          } finally {
>
>