diff --git a/CHANGELOG.md b/CHANGELOG.md index f54fa767..7db4d3e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ All notable changes to this project will be documented in this file. The format ## Unreleased +- Fixed `ResourceWarning` for unclosed file handles ([#1336](https://github.com/jsvine/pdfplumber/issues/1336)). The `PDF` class now emits a `ResourceWarning` (matching Python stdlib behavior) when garbage collected without being properly closed, and recommends using the context manager pattern. - Upgrade `pdfminer.six` from `20251230` to `20260107`. ([07a5ff6](https://github.com/jsvine/pdfplumber/commit/07a5ff6)) ## 0.11.9 — 2026-01-05 diff --git a/README.md b/README.md index f6f1ce3e..de6c3926 100644 --- a/README.md +++ b/README.md @@ -574,6 +574,7 @@ Many thanks to the following users who've contributed ideas, features, and fixes - [Brandon Roberts](https://github.com/brandonrobertz) - [@ennamarie19](https://github.com/ennamarie19) - [Anton Ilin](https://github.com/bronislav) +- [Reg Crampton](https://github.com/rdcrampton) ## Contributing diff --git a/pdfplumber/pdf.py b/pdfplumber/pdf.py index 9c42bc25..09d371f7 100644 --- a/pdfplumber/pdf.py +++ b/pdfplumber/pdf.py @@ -1,6 +1,7 @@ import itertools import logging import pathlib +import warnings from io import BufferedReader, BytesIO from types import TracebackType from typing import Any, Dict, Generator, List, Literal, Optional, Tuple, Type, Union @@ -39,6 +40,7 @@ def __init__( ): self.stream = stream self.stream_is_external = stream_is_external + self._closed = False self.path = path self.pages_to_parse = pages self.laparams = None if laparams is None else LAParams(**laparams) @@ -83,6 +85,21 @@ def open( repair_setting: T_repair_setting = "default", raise_unicode_errors: bool = True, ) -> "PDF": + """Open a PDF file for extraction. + + Recommended usage with a context manager to ensure proper cleanup:: + + with pdfplumber.open("document.pdf") as pdf: + text = pdf.pages[0].extract_text() + + If not using a context manager, call ``pdf.close()`` when done:: + + pdf = pdfplumber.open("document.pdf") + try: + text = pdf.pages[0].extract_text() + finally: + pdf.close() + """ stream: Union[BufferedReader, BytesIO] @@ -122,6 +139,7 @@ def open( raise def close(self) -> None: + self._closed = True self.flush_cache() for page in self.pages: @@ -141,6 +159,19 @@ def __exit__( ) -> None: self.close() + def __del__(self) -> None: + if not getattr(self, "stream_is_external", True) and hasattr(self, "stream"): + if not getattr(self, "_closed", True): + warnings.warn( + f"unclosed PDF file {self.path or ''}." + " Use 'with pdfplumber.open(...)'" + " or call pdf.close() explicitly.", + ResourceWarning, + stacklevel=2, + ) + if not self.stream.closed: + self.stream.close() + @property def pages(self) -> List[Page]: if hasattr(self, "_pages"): diff --git a/tests/test_mcids.py b/tests/test_mcids.py index 006454e6..8220da27 100644 --- a/tests/test_mcids.py +++ b/tests/test_mcids.py @@ -14,42 +14,42 @@ class TestMCIDs(unittest.TestCase): def test_mcids(self): path = os.path.join(HERE, "pdfs/mcid_example.pdf") - pdf = pdfplumber.open(path) - page = pdf.pages[0] - # Check text of MCIDS - mcids = [] - for c in page.chars: - if "mcid" in c: - while len(mcids) <= c["mcid"]: - mcids.append("") - if not mcids[c["mcid"]]: - mcids[c["mcid"]] = c["tag"] + ": " - mcids[c["mcid"]] += c["text"] - assert mcids == [ - "Standard: Test of figures", - "", - "P: 1 ligne", - "P: 2 ligne", - "P: 3 ligne", - "P: 4 ligne", - "P: 0", - "P: 2", - "P: 4", - "P: 6", - "P: 8", - "P: 10", - "P: 12", - "P: Figure 1: Chart", - "", - "P: 1 colonne", - "P: 2 colonne", - "P: 3 colonne", - ] - # Check line and curve MCIDs - line_mcids = set(x["mcid"] for x in page.lines) - curve_mcids = set(x["mcid"] for x in page.curves) - assert all(x["tag"] == "Figure" for x in page.lines) - assert all(x["tag"] == "Figure" for x in page.curves) - assert line_mcids & {1, 14} - assert curve_mcids & {1, 14} - # No rects to test unfortunately! + with pdfplumber.open(path) as pdf: + page = pdf.pages[0] + # Check text of MCIDS + mcids = [] + for c in page.chars: + if "mcid" in c: + while len(mcids) <= c["mcid"]: + mcids.append("") + if not mcids[c["mcid"]]: + mcids[c["mcid"]] = c["tag"] + ": " + mcids[c["mcid"]] += c["text"] + assert mcids == [ + "Standard: Test of figures", + "", + "P: 1 ligne", + "P: 2 ligne", + "P: 3 ligne", + "P: 4 ligne", + "P: 0", + "P: 2", + "P: 4", + "P: 6", + "P: 8", + "P: 10", + "P: 12", + "P: Figure 1: Chart", + "", + "P: 1 colonne", + "P: 2 colonne", + "P: 3 colonne", + ] + # Check line and curve MCIDs + line_mcids = set(x["mcid"] for x in page.lines) + curve_mcids = set(x["mcid"] for x in page.curves) + assert all(x["tag"] == "Figure" for x in page.lines) + assert all(x["tag"] == "Figure" for x in page.curves) + assert line_mcids & {1, 14} + assert curve_mcids & {1, 14} + # No rects to test unfortunately! diff --git a/tests/test_resource_warning.py b/tests/test_resource_warning.py new file mode 100644 index 00000000..9d6412c1 --- /dev/null +++ b/tests/test_resource_warning.py @@ -0,0 +1,77 @@ +#!/usr/bin/env python +import gc +import os +import unittest +import warnings + +import pdfplumber + +HERE = os.path.abspath(os.path.dirname(__file__)) +PATH = os.path.join(HERE, "pdfs/pdffill-demo.pdf") + + +class TestResourceWarning(unittest.TestCase): + def _pdfplumber_warnings(self, warning_list): + """Filter warnings to only those emitted by pdfplumber.""" + return [ + x + for x in warning_list + if issubclass(x.category, ResourceWarning) + and "unclosed PDF file" in str(x.message) + ] + + def test_no_warning_with_context_manager(self): + """Using 'with' should not emit ResourceWarning.""" + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + with pdfplumber.open(PATH) as pdf: + _ = pdf.pages + gc.collect() + pw = self._pdfplumber_warnings(w) + assert len(pw) == 0, f"Unexpected ResourceWarning(s): {pw}" + + def test_no_warning_with_explicit_close(self): + """Calling close() explicitly should not emit ResourceWarning.""" + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + pdf = pdfplumber.open(PATH) + _ = pdf.pages + pdf.close() + del pdf + gc.collect() + pw = self._pdfplumber_warnings(w) + assert len(pw) == 0, f"Unexpected ResourceWarning(s): {pw}" + + def test_warning_without_close(self): + """Forgetting to close should emit ResourceWarning.""" + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + pdf = pdfplumber.open(PATH) + _ = pdf.pages + del _, pdf + gc.collect() + pw = self._pdfplumber_warnings(w) + assert len(pw) == 1 + assert "context manager" in str(pw[0].message).lower() or "close()" in str( + pw[0].message + ) + + def test_external_stream_not_closed(self): + """When the caller passes a stream, pdfplumber should NOT close it.""" + with open(PATH, "rb") as f: + pdf = pdfplumber.open(f) + _ = pdf.pages + del pdf + gc.collect() + assert not f.closed + + def test_external_stream_no_warning(self): + """External streams should not trigger ResourceWarning (caller owns them).""" + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + with open(PATH, "rb") as f: + pdf = pdfplumber.open(f) + _ = pdf.pages + del pdf + gc.collect() + assert len(self._pdfplumber_warnings(w)) == 0 diff --git a/tests/test_utils.py b/tests/test_utils.py index 18e5ce83..bc1f6304 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -30,6 +30,7 @@ def setup_class(self): @classmethod def teardown_class(self): self.pdf.close() + self.pdf_scotus.close() def test_cluster_list(self): a = [1, 2, 3, 4]