You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by oehme <gi...@git.apache.org> on 2018/05/29 16:55:58 UTC
[GitHub] groovy pull request #735: Fix file descriptor leak in AST transform discover...
GitHub user oehme opened a pull request:
https://github.com/apache/groovy/pull/735
Fix file descriptor leak in AST transform discovery
The JVM keeps a cache of URL connections, which keeps
the underlying files open for the lifetime of the VM
unless the `setUseCaches(false)` method is used.
This change makes it possible to reuse the Groovy compiler,
e.g. embedded in the Gradle daemon without preventing files
from being deleted on Windows.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/oehme/groovy master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/groovy/pull/735.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #735
----
commit a6d34536243a42c0599270ad420f7d75438be05b
Author: Stefan Oehme <st...@...>
Date: 2018-05-29T16:53:37Z
Fix file descriptor leak in AST transform discovery
The JVM keeps a cache of URL connections, which keeps
the underlying files open for the lifetime of the VM
unless the `setUseCaches(false)` method is used.
This change makes it possible to reuse the Groovy compiler,
e.g. embedded in the Gradle daemon without preventing files
from being deleted on Windows.
----
---
[GitHub] groovy pull request #735: Fix file descriptor leak in AST transform discover...
Posted by danielsun1106 <gi...@git.apache.org>.
Github user danielsun1106 commented on a diff in the pull request:
https://github.com/apache/groovy/pull/735#discussion_r191637941
--- Diff: src/main/java/org/codehaus/groovy/transform/ASTTransformationVisitor.java ---
@@ -221,7 +222,9 @@ private static void doAddGlobalTransforms(ASTTransformationsContext context, boo
String className;
BufferedReader svcIn = null;
try {
- svcIn = new BufferedReader(new InputStreamReader(service.openStream(), "UTF-8"));
+ URLConnection urlConnection = service.openConnection();
+ urlConnection.setUseCaches(false);
+ svcIn = new BufferedReader(new InputStreamReader(urlConnection.getInputStream(), "UTF-8"));
--- End diff --
Readers will close their underlying Readers(e.g. `BufferedReader`(i.e. `svcIn`) --closes--> `InputStreamReader` --closes--> the stream instance created by `service.openStream()`), and `svcIn` will be closed in the `finally` block, could you please explain how file descriptor leaks?
---
[GitHub] groovy pull request #735: Fix file descriptor leak in AST transform discover...
Posted by oehme <gi...@git.apache.org>.
Github user oehme commented on a diff in the pull request:
https://github.com/apache/groovy/pull/735#discussion_r191647034
--- Diff: src/main/java/org/codehaus/groovy/transform/ASTTransformationVisitor.java ---
@@ -221,7 +222,9 @@ private static void doAddGlobalTransforms(ASTTransformationsContext context, boo
String className;
BufferedReader svcIn = null;
try {
- svcIn = new BufferedReader(new InputStreamReader(service.openStream(), "UTF-8"));
+ URLConnection urlConnection = service.openConnection();
+ urlConnection.setUseCaches(false);
+ svcIn = new BufferedReader(new InputStreamReader(urlConnection.getInputStream(), "UTF-8"));
--- End diff --
As explained in the commit message, the jar will be kept open by the cache in the jvm unless you deactivate that cache. This is meant as a performance optimization and unfortunately it's on by default.
---
[GitHub] groovy pull request #735: Fix file descriptor leaks when using URL connectio...
Posted by oehme <gi...@git.apache.org>.
Github user oehme closed the pull request at:
https://github.com/apache/groovy/pull/735
---