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

Fix resource (FileInputStream) leakage #6

Merged
merged 1 commit into from Apr 18, 2016

Conversation

Projects
None yet
2 participants
@abelgomez
Collaborator

abelgomez commented Apr 13, 2016

There's a resource leakage in the PnmlImport.getDocument(String path) function (FileInputStream is not closed). This makes that files can remain locked after their use in systems like windows (that locks files on use), preventing them from being deleted in the eclipse workspace.

The diff on this pull request git detects all the file has changed becuase of different spacing. To make it clear, the changed code is:

  • Original:
try {
    InputStream in = new BufferedInputStream(new FileInputStream(path));
    // create the builder
    OMXMLParserWrapper builder = OMXMLBuilderFactory.createOMBuilder(in);
    // get the root element
    document = builder.getDocumentElement();
} catch (FileNotFoundException e) {
    throw e;
}
  • Replacement:
try (InputStream in = new BufferedInputStream(new FileInputStream(path));) {
    // create the builder
    OMXMLParserWrapper builder = OMXMLBuilderFactory.createOMBuilder(in);
    // get the root element
    document = builder.getDocumentElement();
} catch (FileNotFoundException e) {
    throw e;
} catch (IOException e) {
    throw new OtherException(e);
}
@abelgomez

This comment has been minimized.

Show comment
Hide comment
@abelgomez

abelgomez Apr 13, 2016

Collaborator

BTW @lhillah ,

I know that I have write permissions on this repository, but since we have not discussed about the policy to commit changes, I have created a PR for you to decide.

Cheers

Collaborator

abelgomez commented Apr 13, 2016

BTW @lhillah ,

I know that I have write permissions on this repository, but since we have not discussed about the policy to commit changes, I have created a PR for you to decide.

Cheers

@lhillah lhillah merged commit f9a9b9f into lip6:master Apr 18, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@lhillah

This comment has been minimized.

Show comment
Hide comment
@lhillah

lhillah Apr 18, 2016

Collaborator

Thanks Abel, merging it. Bug fix release on the way. Update site will now have dedicated folders for each new release as we discussed. Pull requests are fine for now. Let's discuss during this summer commits policy from your side.
Thanks again.

Cheers

Collaborator

lhillah commented Apr 18, 2016

Thanks Abel, merging it. Bug fix release on the way. Update site will now have dedicated folders for each new release as we discussed. Pull requests are fine for now. Let's discuss during this summer commits policy from your side.
Thanks again.

Cheers

lhillah added a commit that referenced this pull request Apr 18, 2016

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