From a793e05c64f9320d4dbc8cdbb8e97692997dab92 Mon Sep 17 00:00:00 2001 From: Daniel King Date: Tue, 22 Sep 2026 12:30:19 -0400 Subject: [PATCH 1/5] Clean up TIFF encoders before their output files close --- Tests/test_file_libtiff.py | 15 +++++++++++++++ docs/releasenotes/13.0.0.rst | 8 ++++++++ src/PIL/TiffImagePlugin.py | 23 +++++++++++++---------- src/libImaging/TiffDecode.c | 1 + 4 files changed, 37 insertions(+), 10 deletions(-) diff --git a/Tests/test_file_libtiff.py b/Tests/test_file_libtiff.py index bdedf94f35c..4283efc8f9c 100644 --- a/Tests/test_file_libtiff.py +++ b/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,20 @@ 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 retains the encoder. Its eventual destruction must + # not seek or write through a file descriptor that may be reused. + 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() diff --git a/docs/releasenotes/13.0.0.rst b/docs/releasenotes/13.0.0.rst index 694093b01ab..eae8fd02222 100644 --- a/docs/releasenotes/13.0.0.rst +++ b/docs/releasenotes/13.0.0.rst @@ -134,6 +134,14 @@ 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. + PNG palettes are no longer padded when saving with the ``bits`` argument ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/PIL/TiffImagePlugin.py b/src/PIL/TiffImagePlugin.py index d41dddcd2f4..a93c073c0ad 100644 --- a/src/PIL/TiffImagePlugin.py +++ b/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: diff --git a/src/libImaging/TiffDecode.c b/src/libImaging/TiffDecode.c index 8a57c523dad..c206998da9d 100644 --- a/src/libImaging/TiffDecode.c +++ b/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; } From b7844f57adfd548a525dae4d220be153ae44e747 Mon Sep 17 00:00:00 2001 From: Daniel King Date: Tue, 22 Sep 2026 12:47:30 -0400 Subject: [PATCH 2/5] Always clean up encoders used by Image.tobytes --- Tests/test_image_tobytes.py | 27 +++++++++++++++++++++++++++ docs/releasenotes/13.0.0.rst | 3 +++ src/PIL/Image.py | 29 ++++++++++++++++------------- 3 files changed, 46 insertions(+), 13 deletions(-) diff --git a/Tests/test_image_tobytes.py b/Tests/test_image_tobytes.py index d32b6c09ba0..7b40f618e9e 100644 --- a/Tests/test_image_tobytes.py +++ b/Tests/test_image_tobytes.py @@ -1,8 +1,35 @@ from __future__ import annotations +from unittest.mock import Mock + +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: + encoder = Mock(spec=ImageFile.PyEncoder) + encoder.encode.return_value = (6, 1, b"pixels") + monkeypatch.setattr(Image, "_getencoder", lambda *args: encoder) + im = Image.new("RGB", (1, 1)) + + if failure in ("setimage", "encode"): + getattr(encoder, failure).side_effect = ValueError(failure) + with pytest.raises(ValueError, match=failure): + im.tobytes() + elif failure == "status": + encoder.encode.return_value = (0, -2, b"") + with pytest.raises(RuntimeError, match="encoder error -2 in tobytes"): + im.tobytes() + else: + assert im.tobytes() == b"pixels" + + encoder.cleanup.assert_called_once_with() diff --git a/docs/releasenotes/13.0.0.rst b/docs/releasenotes/13.0.0.rst index eae8fd02222..55a54c0231d 100644 --- a/docs/releasenotes/13.0.0.rst +++ b/docs/releasenotes/13.0.0.rst @@ -142,6 +142,9 @@ raises an exception. Previously, delayed encoder destruction could access a clos 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 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/PIL/Image.py b/src/PIL/Image.py index 873de8607bf..d793e2eaa35 100644 --- a/src/PIL/Image.py +++ b/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) From d289f6f973e54351ff8aed8a557d750a474deb33 Mon Sep 17 00:00:00 2001 From: Daniel King Date: Tue, 22 Sep 2026 13:50:29 -0400 Subject: [PATCH 3/5] Explain TIFF cleanup regression test --- Tests/test_file_libtiff.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Tests/test_file_libtiff.py b/Tests/test_file_libtiff.py index 4283efc8f9c..6c749a62f4c 100644 --- a/Tests/test_file_libtiff.py +++ b/Tests/test_file_libtiff.py @@ -1276,8 +1276,9 @@ def test_save_error_cleanup(self, tmp_path: Path) -> None: output, format="TIFF", compression="tiff_adobe_deflate" ) - # The traceback retains the encoder. Its eventual destruction must - # not seek or write through a file descriptor that may be reused. + # 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() From 568bf864a028e3edca6c7f9a676dcc7130d33232 Mon Sep 17 00:00:00 2001 From: Daniel King Date: Tue, 22 Sep 2026 14:33:29 -0400 Subject: [PATCH 4/5] Fix release note heading underline --- docs/releasenotes/13.0.0.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/releasenotes/13.0.0.rst b/docs/releasenotes/13.0.0.rst index 55a54c0231d..17aa74152b7 100644 --- a/docs/releasenotes/13.0.0.rst +++ b/docs/releasenotes/13.0.0.rst @@ -135,7 +135,7 @@ 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 From 09b65133f4137aba46734a79853ba5e1a2b4f9bc Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Wed, 23 Sep 2026 11:24:15 +1000 Subject: [PATCH 5/5] Subclass PyEncoder rather than using unittest --- Tests/test_image_tobytes.py | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/Tests/test_image_tobytes.py b/Tests/test_image_tobytes.py index 7b40f618e9e..e6c64397b39 100644 --- a/Tests/test_image_tobytes.py +++ b/Tests/test_image_tobytes.py @@ -1,7 +1,5 @@ from __future__ import annotations -from unittest.mock import Mock - import pytest from PIL import Image, ImageFile @@ -16,20 +14,39 @@ def test_sanity() -> None: @pytest.mark.parametrize("failure", (None, "setimage", "encode", "status")) def test_encoder_cleanup(failure: str | None, monkeypatch: pytest.MonkeyPatch) -> None: - encoder = Mock(spec=ImageFile.PyEncoder) - encoder.encode.return_value = (6, 1, b"pixels") - monkeypatch.setattr(Image, "_getencoder", lambda *args: encoder) + 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"): - getattr(encoder, failure).side_effect = ValueError(failure) with pytest.raises(ValueError, match=failure): im.tobytes() elif failure == "status": - encoder.encode.return_value = (0, -2, b"") with pytest.raises(RuntimeError, match="encoder error -2 in tobytes"): im.tobytes() else: assert im.tobytes() == b"pixels" - encoder.cleanup.assert_called_once_with() + assert cleanup_called