Repository navigation
Conversation
_as_blocks() appended each block tuple to the result list with
LIST_APPEND(), which does not drop the reference returned by
PyTuple_New(). Every tuple (and its float, str and int items) therefore
had a refcount one too high and was never freed: get_text('blocks')
leaked ~275 bytes per block on every call.
Use LIST_APPEND_DROP() like the other extractors, and remove
LIST_APPEND(), which has no other caller.
Checks that get_text('blocks') returns block tuples owned by the result
list only and that repeated calls do not grow traced memory.
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
Author
|
I have read the CLA Document and I hereby sign the CLA |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Follow-up to #5067 / #5074, which fixed the leaked text reference in the
same function: the block tuple itself still leaks.
_as_blocks()appended each block tuple to the result list withLIST_APPEND(), which keeps the reference returned byPyTuple_New(). Everytuple returned by
Page.get_text("blocks")/TextPage.extractBLOCKS()—with its floats, text and ints — therefore leaked on every call (from ~275 bytes
to ~1.9 KB per block, depending on the text length).
Changes:
src/extra.i: useLIST_APPEND_DROP()in_as_blocks(), like the otherextractors (
JM_append_word(),JM_make_textpage_dict()…), and removeLIST_APPEND(), which has no other caller.tests/test_memory.py:test_extractblocks_refcount()checks that eachblock tuple has the same refcount as a tuple held by a list only, and that
200 calls of
get_text("blocks")do not grow tracemalloc's traced memory.The output of
get_text("blocks")is unchanged (see measures below).Measures
Same script, same page (
tests/resources/2201.00069.pdf, page 0, 15 blocks),Linux aarch64 (Docker
python:3.14), both wheels built from source with thesame MuPDF build —
main@ 28b271b vs. this branch:mainget_text("blocks"), 2000 calls, tracemalloc growthget_text("words" / "dict" / "text"), 2000 callstest_extractblocks_refcountassert 3 == 2)get_text("blocks")output is byte-identical between both builds on all189 PDFs of
tests/resources/(1485 pages, 26798 blocks). The new testalso fails against the released
pymupdf==2.1.0wheel (macOS arm64).