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

ImportControlLoader does not close InputStream and leaks filehandles when xml is malformed #3962

Closed
gaganis opened this Issue Mar 8, 2017 · 3 comments

Comments

Projects
None yet
3 participants
@gaganis
Contributor

gaganis commented Mar 8, 2017

Overview

I have been looking into this checkstyle plugin related gradle issue gradle/gradle#1416. One of the issues the original author has is a problem with the import-control.xml file remaining open after gradle has finished execution.

I have managed to replicate the issue on my Linux box using the provided project and checkstyle configuration.

I have tracked the problem to be in ImportControlLoader which opens an InputStream but not closes it in case there is a parse exception.

Reproduction steps

Setup project as described in gradle/gradle#1416

$ $JAVA_HOME/bin/jps                      
26515 RemoteMavenServer
28548 Main
17143 Jps
15103 Launcher
$ /home/gaganis/workspace/gradle-development/gradle/bin/gradle clean build --stacktrace --refresh-dependencies

...
Execution failed for task ':checkstyleMain'.
> Unable to create Root Module: configLocation {/home/gaganis/programming/gradle/issue-1416/project/config/checkstyle/checkstyle.xml}, classpath {/home/gaganis/programming/gradle/issue-1416/project/build/classes/main:/home/gaganis/programming/gradle/issue-1416/project/build/resources/main}.

...
Caused by: com.puppycrawl.tools.checkstyle.api.CheckstyleException: unable to parse file:/home/gaganis/programming/gradle/issue-1416/project/config/checkstyle/import-control.xml - The content of element type "import-control" must match "((allow|disallow)*,subpackage*)".
        at com.puppycrawl.tools.checkstyle.checks.imports.ImportControlLoader.load(ImportControlLoader.java:194)
        at com.puppycrawl.tools.checkstyle.checks.imports.ImportControlLoader.load(ImportControlLoader.java:175)
        at com.puppycrawl.tools.checkstyle.checks.imports.ImportControlCheck.setFile(ImportControlCheck.java:185)
        ... 107 more
lsof -n|grep checkstyle                                                                                     
java      17293                gaganis  221r      REG                8,5       371    1791156 /home/gaganis/programming/gradle/issue-1416/project/config/checkstyle/import-control.xml
...

When checkstyle library fails to read import-control.xml for whatever reason it should not leave unclosed streams in the containing jvm.


I have tries applying a fix using a try-with-resources construct but I got stuck because it drops coverage in cobertura. I have researched this quite a bit cobertura/cobertura#289 but was unable to find a solution.

gaganis added a commit to gaganis/checkstyle that referenced this issue Mar 8, 2017

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Mar 8, 2017

Member

it should not leave unclosed streams in the containing jvm

Once checkstyle ends in gradle, doesn't the JVM terminate and close the stream then?

Member

rnveach commented Mar 8, 2017

it should not leave unclosed streams in the containing jvm

Once checkstyle ends in gradle, doesn't the JVM terminate and close the stream then?

@gaganis

This comment has been minimized.

Show comment
Hide comment
@gaganis

gaganis Mar 8, 2017

Contributor

Gradle uses a daemon that runs in the background and is reused between successive builds.

Contributor

gaganis commented Mar 8, 2017

Gradle uses a daemon that runs in the background and is reused between successive builds.

gaganis added a commit to gaganis/checkstyle that referenced this issue Mar 16, 2017

gaganis added a commit to gaganis/checkstyle that referenced this issue Mar 16, 2017

gaganis added a commit to gaganis/checkstyle that referenced this issue Mar 20, 2017

gaganis added a commit to gaganis/checkstyle that referenced this issue Mar 20, 2017

gaganis added a commit to gaganis/checkstyle that referenced this issue Mar 20, 2017

gaganis added a commit to gaganis/checkstyle that referenced this issue Mar 21, 2017

gaganis added a commit to gaganis/checkstyle that referenced this issue Mar 21, 2017

gaganis added a commit to gaganis/checkstyle that referenced this issue Mar 21, 2017

gaganis added a commit to gaganis/checkstyle that referenced this issue Mar 21, 2017

gaganis added a commit to gaganis/checkstyle that referenced this issue Mar 21, 2017

gaganis added a commit to gaganis/checkstyle that referenced this issue Mar 21, 2017

gaganis added a commit to gaganis/checkstyle that referenced this issue Mar 21, 2017

gaganis added a commit to gaganis/checkstyle that referenced this issue Mar 21, 2017

gaganis added a commit to gaganis/checkstyle that referenced this issue Mar 21, 2017

gaganis added a commit to gaganis/checkstyle that referenced this issue Mar 21, 2017

gaganis added a commit to gaganis/checkstyle that referenced this issue Mar 21, 2017

rnveach added a commit that referenced this issue Mar 22, 2017

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Mar 22, 2017

Member

Fix is merged

Member

rnveach commented Mar 22, 2017

Fix is merged

@rnveach rnveach closed this Mar 22, 2017

@rnveach rnveach added this to the 7.7 milestone Mar 22, 2017

SergeyDzyuba added a commit to SergeyDzyuba/checkstyle that referenced this issue Mar 22, 2017

@romani romani changed the title from ImportControlLoader does not close InputStream and leaks filehandles when xml is malformed. to ImportControlLoader does not close InputStream and leaks filehandles when xml is malformed Mar 26, 2017

GitToasterhub added a commit to GitToasterhub/checkstyle that referenced this issue Mar 26, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment