search

jai-imageio / jai-imageio-jpeg2000

JPEG2000 support for Java Advanced Imaging Image I/O Tools API
Other
73 stars 56 forks source link

J2KImageReader: Add dispose method to fix an extreme memory leak #19

Closed rleigh-codelibre closed 3 years ago

rleigh-codelibre commented 7 years ago

See https://github.com/jai-imageio/jai-imageio-core/pull/2 for original PR.

rleigh-codelibre commented 7 years ago

The ConverterTest.testname() test fails here:

                        ImageIO.read(f);

The .read(f) call fails because the stream f was closed. [Because it was used by multiple J2KImageReader instances?]

When passing an open stream to J2KImageReader, is it acceptable for the reader to close it, or does that responsibility lie with the caller which created it and passed it the stream (and is implicitly the owner of the stream)? Or is ownership of the stream fully transferred to the reader? It looks here like it breaks some fairly fundamental functionality of ImageIO, unless the unit test needs adjusting to take account of the changed behaviour?

/cc @melissalinkert @jmabrey

melissalinkert commented 7 years ago

As suggested on a previous iteration of this change, you might try calling iis.flush() instead of iis.close() to see if that resolves memory issues without affecting tests (see https://github.com/jai-imageio/jai-imageio-jpeg2000/pull/2#issuecomment-76875274). Based upon the previous PR discussions, I'd assume that responsibility for closing the stream is with the caller and not the reader.

stain commented 6 years ago

Agreed with @melissalinkert , it should not be closing something it didn't open.

rleigh-codelibre commented 3 years ago

I'm not doing work on this going forward. If anyone wants to pick this up, please feel free to do so.