Close many potential resource leaks #161

Merged
merged 3 commits into from Aug 26, 2014

Conversation

Projects
None yet
2 participants
@jwcranford

This pull request closes many potential resource leaks by closing all resources in finally blocks.

It also includes a patch to build.xml to read the saxon version from the Version class instead of hardcoding it in the build file.

I recommend ignoring whitespace when viewing the differences in this pull request. Github doesn't ignore whitespace when showing the differences, but you can do that from the command-line:

git diff -b -w 986694d 13fa597

or my favorite

git diff -b -w --color 986694d 13fa597

Additional notes:
src/com/xmlcalabash/io/ReadableData.java

  • changed so that underlying stream only gets closed manually when necessary

src/com/xmlcalabash/util/Base64.java

  • Patched Base64 to prevent NullPointerExceptions and to log any exceptions
    on close (instead of silently swallowing them). The following are still viable upgrade paths for Base64 in the future:
    • Upgrade to the next version (2.3) of
      Base64 (http://iharder.net/base64). It's not backwards compatible, as it
      throws exceptions instead of silently swallowing them, so it's not a
      drop-in replacement. This patch doesn't make upgrading to the next version any more difficult.
    • Upgrade to Java 8 and java.util.Base64. API not compatible, so some
      porting would be necessary.

Jonathan W. Cranford added some commits Jun 13, 2014

Jonathan W. Cranford
Fixed many possible resource leaks
by putting all close() calls in finally blocks
Jonathan Cranford
build.xml now reads saxon version from Version class
I think this will work with all versions of saxon >= 9.3, but I only
tested it with Saxon 9.5, so I put it in the saxon95 target.

Also, it requires Ant 1.7.0+ now because of some of the features used.
@jwcranford

This comment has been minimized.

Show comment
Hide comment
@jwcranford

jwcranford Jun 20, 2014

One other note: As a sanity check, I ran the suite of conformance tests against the code base with these changes, and I got the same results as against version 1.0.18.

One other note: As a sanity check, I ran the suite of conformance tests against the code base with these changes, and I got the same results as against version 1.0.18.

@ndw ndw merged commit 13fa597 into ndw:saxon95 Aug 26, 2014

@ndw

This comment has been minimized.

Show comment
Hide comment
@ndw

ndw Aug 26, 2014

Owner

Thanks!

Owner

ndw commented Aug 26, 2014

Thanks!

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