diff --git a/CHANGELOG.md b/CHANGELOG.md index f9d161e..314eb19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ All notable changes to this project will be documented in this file. The format ## [Unreleased] ### Fixed +- Fix `PDF.close()` closing the wrong `Page` instances: it called `flush_cache()` (which clears the cached `_pages` list) before iterating `self.pages`, so that iteration re-parsed and closed a fresh, never-otherwise-referenced set of pages instead of the ones the caller actually used — leaving the real pages' `_objects`/`_layout`/`get_textmap` caches unreleased. This caused unbounded-seeming memory growth in long-lived processes that open many PDFs sequentially. ([#1339](https://github.com/jsvine/pdfplumber/issues/1339), related: [#1229](https://github.com/jsvine/pdfplumber/issues/1229), [#1189](https://github.com/jsvine/pdfplumber/issues/1189)) - Initialize PDFium's form environment in `get_page_image` so that filled AcroForm field content is included when rendering pages via `Page.to_image()`. ([#1367](https://github.com/jsvine/pdfplumber/issues/1367)) - Fix `make venv`, which created the virtual environment at `venv/` but then installed into `${VENV}` (default `.venv/`), causing the target to fail on a fresh checkout (h/t @soodoku). ([ec96f72](https://github.com/jsvine/pdfplumber/commit/ec96f72)) - Include the wrapped exception's class name in `PdfminerException`'s message when `pdfminer.six` raises an exception without one (e.g. `PDFPasswordIncorrect`), which previously surfaced as a blank error message. diff --git a/README.md b/README.md index dc23ac9..b063835 100644 --- a/README.md +++ b/README.md @@ -577,6 +577,7 @@ Many thanks to the following users who've contributed ideas, features, and fixes - [Sebastian Cao](https://github.com/cycsmail) - [Kaspar Naraghi](https://github.com/kaninaba94) - [Siddharth Gaur](https://github.com/siddharthgaur1) +- [Timothy O'Hare](https://github.com/timothyohare) ## Contributing diff --git a/pdfplumber/pdf.py b/pdfplumber/pdf.py index f3d2dc6..3628c72 100644 --- a/pdfplumber/pdf.py +++ b/pdfplumber/pdf.py @@ -122,11 +122,16 @@ def open( raise def close(self) -> None: - self.flush_cache() - + # Must close the pages *before* flush_cache() clears `_pages`; the + # `pages` property re-parses (and returns a fresh, never-otherwise- + # referenced set of pages) if `_pages` is missing, so flushing first + # means the pages actually used by the caller never get their heavy + # caches (`_objects`, `_layout`, `get_textmap`) released. See #1339. for page in self.pages: page.close() + self.flush_cache() + if not self.stream_is_external: self.stream.close() diff --git a/tests/test_issues.py b/tests/test_issues.py index e0823f2..ae8c7f3 100644 --- a/tests/test_issues.py +++ b/tests/test_issues.py @@ -338,3 +338,26 @@ def test_pr_1195(self): ): for _ in pdf.annots: pass + + def test_issue_1339(self): + """ + PDF.close() closed the wrong Page instances: flush_cache() ran + first and deleted the cached `_pages` list, so the subsequent + `for page in self.pages: page.close()` re-parsed and closed a + fresh, never-otherwise-referenced set of pages instead of the ones + the caller actually used -- leaving the real pages' `_objects` / + `_layout` / `get_textmap` caches unreleased after close(). + https://github.com/jsvine/pdfplumber/issues/1339 + """ + path = os.path.join(HERE, "pdfs/issue-33-lorem-ipsum.pdf") + pdf = pdfplumber.open(path) + pages = pdf.pages + for page in pages: + page.extract_text() + assert hasattr(page, "_objects") + + pdf.close() + + for page in pages: + assert not hasattr(page, "_objects") + assert page.get_textmap.cache_info().currsize == 0