red6 / pdfcompare

A simple Java library to compare two PDF files
Apache License 2.0
220 stars 66 forks source link

Resource leak: InputStreams are not closed #145

Closed S-Gaertner closed 7 months ago

S-Gaertner commented 7 months ago

InputStreams opened in the following lines of PdfComparator are not closed anywhere:

Especially, the RandomAccessReadBuffers in lines 392 and 393, where those InputStreams are used, do not close their underlying InputStreams upon their own closure.

This can lead to resource leaks and file locking, depending on the platform-specific InputStream implementation. At some point in time the garbage collector will close the left over InputStreams, which can lead to undesirable behaviour due to its non-deterministic nature.

In case InputStreams are created internally in the library, they should be closed when not needed anymore. The only case where the InputStreams should not be closed is when they are created externally by the user and passed into the library using the PdfComparator(final InputStream expectedPdfIS, final InputStream actualPdfIS) constructor.

finsterwalder commented 7 months ago

Good catch! I was under the assumption, that the RandomAccessReadBuffer would close the "underlying" inputStream. Will change that.

finsterwalder commented 7 months ago

Version 1.2.1 is in the making.

S-Gaertner commented 7 months ago

Wow, that was quick! Thanks a lot 👍

finsterwalder commented 7 months ago

1.2.1 was released and should land in maven central within the next 1-2 hours. With such a specific error description, it was easy to fix.

S-Gaertner commented 7 months ago

Unfortunately your fix does not work. I believe now you ask your InputStreamSuppliers to get yet another InputStream in lines 393 and 395, which you immediately close. However, the InputStreams you got in the immediately preceding lines are still not closed.

finsterwalder commented 7 months ago

You are right. I was too quick. get() does not just "get" the stream, but actually creates it, since it's a Supplier. Version 1.2.2 should fix it properly now. :-)

S-Gaertner commented 7 months ago

Thank you again for your stunningly quick resolution! I can confirm that the fix works now, therefore I'll close this issue.