Clean up encoders deterministically on success and failure

Tests/test_file_libtiff.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import base64
+import gc
 import io
 import itertools
 import os
@@ -1268,6 +1269,21 @@ def test_save_zero(self, compression: str | None, tmp_path: Path) -> None:
         with pytest.raises(ValueError, match="cannot write empty image"):
             im.save(out, compression=compression)
 
+    def test_save_error_cleanup(self, tmp_path: Path) -> None:
+        with (tmp_path / "temp.tif").open("w+b", buffering=0) as output:
+            with pytest.raises(ValueError, match="cannot write empty image") as exc:
+                Image.new("RGB", (0, 0)).save(
+                    output, format="TIFF", compression="tiff_adobe_deflate"
+                )
+
+            # The traceback keeps the encoder alive. Collecting it used to move
+            # the file position, corrupting unrelated reads if the fd was reused.
+            # Cleanup must happen during save, not later during GC.
+            output.seek(1234)
+            del exc
+            gc.collect()
+            assert output.tell() == 1234
+
     @pytest.mark.skipif(sys.platform != "win32", reason="Checks a Windows handle limit")
     def test_save_many_compressed(self, tmp_path: Path) -> None:
         im = hopper()

Tests/test_image_tobytes.py

@@ -1,8 +1,52 @@
 from __future__ import annotations
 
+import pytest
+
+from PIL import Image, ImageFile
+
 from .helper import hopper
 
 
 def test_sanity() -> None:
     data = hopper().tobytes()
     assert isinstance(data, bytes)
+
+
+@pytest.mark.parametrize("failure", (None, "setimage", "encode", "status"))
+def test_encoder_cleanup(failure: str | None, monkeypatch: pytest.MonkeyPatch) -> None:
+    cleanup_called = False
+
+    class TestPyEncoder(ImageFile.PyEncoder):
+        def encode(self, bufsize: int) -> tuple[int, int, bytes]:
+            if failure == "encode":
+                raise ValueError(failure)
+            if failure == "status":
+                return (0, -2, b"")
+            return (6, 1, b"pixels")
+
+        def setimage(
+            self,
+            im: Image.core.ImagingCore,
+            extents: tuple[int, int, int, int] | None = None,
+        ) -> None:
+            if failure == "setimage":
+                raise ValueError(failure)
+            return super().setimage(im, extents)
+
+        def cleanup(self) -> None:
+            nonlocal cleanup_called
+            cleanup_called = True
+
+    im = Image.new("RGB", (1, 1))
+
+    monkeypatch.setattr(Image, "ENCODERS", {"raw": TestPyEncoder})
+    if failure in ("setimage", "encode"):
+        with pytest.raises(ValueError, match=failure):
+            im.tobytes()
+    elif failure == "status":
+        with pytest.raises(RuntimeError, match="encoder error -2 in tobytes"):
+            im.tobytes()
+    else:
+        assert im.tobytes() == b"pixels"
+
+    assert cleanup_called

docs/releasenotes/13.0.0.rst

@@ -134,6 +134,17 @@ escape sequences.
 Other changes
 =============
 
+Clean up TIFF encoders before closing their output files
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+TIFF saving now explicitly cleans up the libtiff encoder, including when saving
+raises an exception. Previously, delayed encoder destruction could access a closed
+file descriptor after it had been reused for another file, disturbing that file's
+position or contents.
+
+:py:meth:`~PIL.Image.Image.tobytes` also now explicitly cleans up its encoder on
+both success and failure, rather than relying on object destruction.
+
 PNG palettes are no longer padded when saving with the ``bits`` argument
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 

src/PIL/Image.py

@@ -883,21 +883,24 @@ def tobytes(self, encoder_name: str = "raw", *args: Any) -> bytes:
 
         # unpack data
         e = _getencoder(self.mode, encoder_name, encoder_args)
-        e.setimage(self.im, (0, 0, *self.size))
+        try:
+            e.setimage(self.im, (0, 0, *self.size))
 
-        from . import ImageFile
+            from . import ImageFile
+
+            bufsize = max(ImageFile.MAXBLOCK, self.size[0] * 4)  # see RawEncode.c
 
-        bufsize = max(ImageFile.MAXBLOCK, self.size[0] * 4)  # see RawEncode.c
-
-        output = []
-        while True:
-            bytes_consumed, errcode, data = e.encode(bufsize)
-            output.append(data)
-            if errcode:
-                break
-        if errcode < 0:
-            msg = f"encoder error {errcode} in tobytes"
-            raise RuntimeError(msg)
+            output = []
+            while True:
+                bytes_consumed, errcode, data = e.encode(bufsize)
+                output.append(data)
+                if errcode:
+                    break
+            if errcode < 0:
+                msg = f"encoder error {errcode} in tobytes"
+                raise RuntimeError(msg)
+        finally:
+            e.cleanup()
 
         return b"".join(output)
 

src/PIL/TiffImagePlugin.py

@@ -1998,16 +1998,19 @@ def _save(im: Image.Image, fp: IO[bytes], filename: str | bytes) -> None:
         tags.sort()
         a = (rawmode, compression, _fp, filename, tags, types)
         encoder = Image._getencoder(im.mode, "libtiff", a, encoderconfig)
-        encoder.setimage(im.im, (0, 0, *im.size))
-        while True:
-            errcode, data = encoder.encode(ImageFile.MAXBLOCK)[1:]
-            if not _fp:
-                fp.write(data)
-            if errcode:
-                break
-        if errcode < 0:
-            msg = f"encoder error {errcode} when writing image file"
-            raise OSError(msg)
+        try:
+            encoder.setimage(im.im, (0, 0, *im.size))
+            while True:
+                errcode, data = encoder.encode(ImageFile.MAXBLOCK)[1:]
+                if not _fp:
+                    fp.write(data)
+                if errcode:
+                    break
+            if errcode < 0:
+                msg = f"encoder error {errcode} when writing image file"
+                raise OSError(msg)
+        finally:
+            encoder.cleanup()
 
     else:
         for tag in blocklist:

src/libImaging/TiffDecode.c

@@ -954,6 +954,7 @@ ImagingLibTiffEncodeCleanup(ImagingCodecState state) {
         // that is fine, as it does not close the file
         TIFFClose(tiff);
     }
+    clientstate->tiff = NULL;
     return 0;
 }