diff --git a/src/extra.i b/src/extra.i index 0e580df9b..2555bdfa7 100644 --- a/src/extra.i +++ b/src/extra.i @@ -736,13 +736,6 @@ static int LIST_APPEND_DROP(PyObject *list, PyObject *item) return rc; } -static int LIST_APPEND(PyObject *list, PyObject *item) -{ - if (!list || !PyList_Check(list) || !item) return -2; - int rc = PyList_Append(list, item); - return rc; -} - static int DICT_SETITEM_DROP(PyObject *dict, PyObject *key, PyObject *value) { if (!dict || !PyDict_Check(dict) || !key || !value) return -2; @@ -3531,7 +3524,7 @@ int _as_blocks(fz_stext_block *block, fz_rect tp_rect, PyObject *lines, int bloc PyTuple_SET_ITEM(litem, 4, text); PyTuple_SET_ITEM(litem, 5, Py_BuildValue("i", block_n)); PyTuple_SET_ITEM(litem, 6, Py_BuildValue("i", block->type)); - LIST_APPEND(lines, litem); + LIST_APPEND_DROP(lines, litem); } text = NULL; block = block->next; diff --git a/tests/test_memory.py b/tests/test_memory.py index ab55b0861..e4a051771 100644 --- a/tests/test_memory.py +++ b/tests/test_memory.py @@ -356,3 +356,44 @@ def test_4751(): print(f'test_4751(): Running: {command!r}', flush=1) print(f'test_4751(): With: {env_extra=}', flush=1) subprocess.run(command, shell=1, check=1, env=os.environ | env_extra) + + +def test_extractblocks_refcount(): + ''' + Check that get_text('blocks') does not leak its block tuples. + + _as_blocks() used to append each tuple to the result list without + dropping its own reference, so every block (tuple, floats, str, ints) + leaked on every call. + ''' + path = os.path.normpath(f'{__file__}/../../tests/resources/2201.00069.pdf') + with pymupdf.open(path) as document: + page = document[0] + blocks = page.get_text('blocks') + assert blocks + + # Each tuple must be referenced by the result list only, exactly like + # a tuple that Python itself put into a list. + control = [tuple(list(blocks[0]))] + expected = sys.getrefcount(control[0]) + for i in range(len(blocks)): + assert sys.getrefcount(blocks[i]) == expected, f'{i=} {blocks[i][:4]=}' + + # Memory must stay flat over repeated calls. + import tracemalloc + for _ in range(10): + page.get_text('blocks') + gc.collect() + tracemalloc.start() + try: + before = tracemalloc.get_traced_memory()[0] + n = 200 + for _ in range(n): + page.get_text('blocks') + gc.collect() + growth = tracemalloc.get_traced_memory()[0] - before + finally: + tracemalloc.stop() + print(f'test_extractblocks_refcount(): {len(blocks)=} {n=} {growth=}.') + # Leaking, growth was ~275 bytes per block per call (> 1 MB here). + assert growth < 50_000, f'{growth=}'