Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ShadowCopyAction still isn't closing all input streams #408

Closed
roxchkplusony opened this issue Oct 2, 2018 · 4 comments
Closed

ShadowCopyAction still isn't closing all input streams #408

roxchkplusony opened this issue Oct 2, 2018 · 4 comments

Comments

@roxchkplusony
Copy link
Contributor

Please check the User Guide before submitting "how do I do 'x'?" questions!

Shadow Version

2.0.4, 4.0.1

Gradle Version

4.10

Expected Behavior

Everyplace the code is calling archive.getInputStream or otherwise opening a stream should be closed. For example, copyArchiveEntry should close the inputStream from archive.getInputStream as IOUtils.copyLarge does not.

Actual Behavior

Our build process runs out of file handles while building our shadow jars.

Gradle Build Script(s)

Content of Shadow JAR (jar tf <jar file> - post link to GIST if too long)

@roxchkplusony
Copy link
Contributor Author

This patch below follows the patterns of the file including the change meant to address #364. However, in the case of transform() I would recommend just fixing the innermost transform() implementation and reducing the calling functions. You would not break anything, not lose any clarity, and you would be better protected against future problems. Add a comment if it helps. I wouldn't make the same recommendation for an API method, but in this case you always want the stream closed.

Index: src/main/groovy/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.groovy
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/main/groovy/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.groovy	(date 1538332764000)
+++ src/main/groovy/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.groovy	(date 1538505703000)
@@ -251,19 +251,22 @@
         private void processArchive(FileCopyDetails fileDetails) {
             stats.startJar()
             ZipFile archive = new ZipFile(fileDetails.file)
-            List<ArchiveFileTreeElement> archiveElements = archive.entries.collect {
-                new ArchiveFileTreeElement(new RelativeArchivePath(it, fileDetails))
-            }
-            Spec<FileTreeElement> patternSpec = patternSet.getAsSpec()
-            List<ArchiveFileTreeElement> filteredArchiveElements = archiveElements.findAll { ArchiveFileTreeElement archiveElement ->
-                patternSpec.isSatisfiedBy(archiveElement)
-            }
-            filteredArchiveElements.each { ArchiveFileTreeElement archiveElement ->
-                if (archiveElement.relativePath.file) {
-                    visitArchiveFile(archiveElement, archive)
-                }
-            }
-            archive.close()
+            try {
+                List<ArchiveFileTreeElement> archiveElements = archive.entries.collect {
+                    new ArchiveFileTreeElement(new RelativeArchivePath(it, fileDetails))
+                }
+                Spec<FileTreeElement> patternSpec = patternSet.getAsSpec()
+                List<ArchiveFileTreeElement> filteredArchiveElements = archiveElements.findAll { ArchiveFileTreeElement archiveElement ->
+                    patternSpec.isSatisfiedBy(archiveElement)
+                }
+                filteredArchiveElements.each { ArchiveFileTreeElement archiveElement ->
+                    if (archiveElement.relativePath.file) {
+                        visitArchiveFile(archiveElement, archive)
+                    }
+                }
+            } finally {
+                archive.close()
+            }
             stats.finishJar()
         }
 
@@ -408,7 +411,12 @@
         }
 
         private void transform(FileCopyDetails details) {
-            transform(details, details.file.newInputStream())
+            InputStream is = details.file.newInputStream()
+            try {
+                transform(details, is)
+            } finally {
+                is.close()
+            }
         }
 
         private void transform(FileTreeElement element, InputStream is) {

@johnrengelman
Copy link
Collaborator

Any chance you can submit this as a PR?

@roxchkplusony
Copy link
Contributor Author

roxchkplusony commented Oct 7, 2018 via email

@roxchkplusony
Copy link
Contributor Author

@johnrengelman all ready to go.

johnrengelman added a commit that referenced this issue Oct 27, 2018
[Fixes #408] always close input streams we create
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants