Do not close provided file handles with libtiff #7199
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #7042
libtiff closes file handles, but Pillow might still want access afterwards to seek to other frames in the file. To resolve this, TiffImagePlugin duplicates the file handle.
Pillow/src/PIL/TiffImagePlugin.py
Lines 1256 to 1258 in 9a560c7
#5936 had a problem with too many open files, so the following code was added to ensure that if libtiff did not close the file, Pillow would still close the file.
Pillow/src/PIL/TiffImagePlugin.py
Lines 1308 to 1312 in 9a560c7
#7042 doesn't like this behaviour, feeling that it is not thread-safe to close a file handle more than once.
Investigating how exactly libtiff closes file handles, I found that it is
_tiffCloseProc
that closes the file handle. This is called byTIFFClose
, which we call fromPillow/src/libImaging/TiffDecode.c
Line 723 in 9a560c7
Looking at the code for
TIFFClose
, something becomes apparent - if we didn't want to call_tiffCloseProc
when a file handle is in use, then we could just callTIFFCleanup
instead ofTIFFClose
. Then we don't need to duplicate the file handle and don't need to try and close the duplicated file handle. Testing, this shouldn't break #5936, and passes #7042's reproduction (although that is not necessarily indicative).