Fix and test memory leak in Document.save() etc.

.github/workflows/test-valgrind.yml

@@ -96,6 +96,7 @@ jobs:
                     f'./pylocal/bin/python {leaf}/tests/run_compound.py valgrind --suppressions={leaf}/valgrind.supp --error-exitcode=100 --errors-for-leak-kinds=none --fullpath-after= ./pylocal/bin/python -m pytest -s -vv {leaf}',
                     env_extra=dict(
                         PYTHONMALLOC='malloc',
+                        PYMUPDF_RUNNING_ON_VALGRIND='1',
                         ),
                     )
 

.github/workflows/test.yml

@@ -46,6 +46,6 @@ jobs:
             CIBW_SKIP: "pp* *i686 *-musllinux_* cp36*"
 
             # Get cibuildwheel to run pytest with each wheel.
-            CIBW_TEST_REQUIRES: "fontTools pytest"
+            CIBW_TEST_REQUIRES: "fontTools pytest psutil"
             CIBW_TEST_COMMAND: "python {project}/tests/run_compound.py pytest -s {project}/tests"
             CIBW_BUILD_VERBOSITY: 3

.github/workflows/test_mupdf-master-branch.yml

@@ -48,6 +48,6 @@ jobs:
             CIBW_SKIP: "pp* *i686 *-musllinux_* cp36*"
 
             # Get cibuildwheel to run pytest with each wheel.
-            CIBW_TEST_REQUIRES: "fontTools pytest"
+            CIBW_TEST_REQUIRES: "fontTools pytest psutil"
             CIBW_TEST_COMMAND: "python {project}/tests/run_compound.py pytest -s {project}/tests"
             CIBW_BUILD_VERBOSITY: 3

.github/workflows/test_mupdf-release-branch.yml

@@ -49,6 +49,6 @@ jobs:
             CIBW_SKIP: "pp* *i686 *-musllinux_* cp36*"
 
             # Get cibuildwheel to run pytest with each wheel.
-            CIBW_TEST_REQUIRES: "fontTools pytest"
+            CIBW_TEST_REQUIRES: "fontTools pytest psutil"
             CIBW_TEST_COMMAND: "python {project}/tests/run_compound.py pytest -s {project}/tests"
             CIBW_BUILD_VERBOSITY: 3

.github/workflows/test_quick.yml

@@ -44,6 +44,6 @@ jobs:
             CIBW_SKIP: "pp* *i686 *-musllinux_* cp36* *win32*"
 
             # Get cibuildwheel to run pytest with each wheel.
-            CIBW_TEST_REQUIRES: "fontTools pytest"
+            CIBW_TEST_REQUIRES: "fontTools pytest psutil"
             CIBW_TEST_COMMAND: "python {project}/tests/run_compound.py pytest -s {project}/tests"
             CIBW_BUILD_VERBOSITY: 3

pyproject.toml

@@ -1,5 +1,5 @@
 [build-system]
-requires = ["libclang", "swig", "setuptools"]
+requires = ["libclang", "swig", "setuptools", "psutil"]
 
 # See pep-517.
 #

scripts/gh_release.py

@@ -277,7 +277,7 @@ def set_cibuild_test():
         if inputs_skeleton:
             env_set('CIBW_TEST_COMMAND', 'python {project}/scripts/gh_release.py test {project} {package}')
         else:
-            env_set('CIBW_TEST_REQUIRES', 'fontTools pytest')
+            env_set('CIBW_TEST_REQUIRES', 'fontTools pytest psutil')
             env_set('CIBW_TEST_COMMAND', 'python {project}/tests/run_compound.py pytest -s {project}/tests')
 
     pymupdf_dir = os.path.abspath( f'{__file__}/../..')

src/extra.i

@@ -1426,9 +1426,10 @@ static void JM_bytesio_seek(fz_context* ctx, void* opaque, int64_t off, int when
     PyObject* bio = (PyObject*) opaque;
     PyObject* name = PyUnicode_FromString("seek");
     PyObject* pos = PyLong_FromUnsignedLongLong((unsigned long long) off);
-    PyObject_CallMethodObjArgs(bio, name, pos, whence, nullptr);
+    PyObject* rc = PyObject_CallMethodObjArgs(bio, name, pos, whence, nullptr);
+    Py_XDECREF(rc);
     std::string e;
-    PyObject* rc = PyErr_Occurred();
+    rc = PyErr_Occurred();
     if (rc)
     {
         e = "Could not write to Py file obj: " + repr(bio);
@@ -1453,9 +1454,10 @@ static void JM_bytesio_write(fz_context* ctx, void* opaque, const void* data, si
     PyObject* bio = (PyObject*) opaque;
     PyObject* b = PyBytes_FromStringAndSize((const char*) data, (Py_ssize_t) len);
     PyObject* name = PyUnicode_FromString("write");
-    PyObject_CallMethodObjArgs(bio, name, b, nullptr);
+    PyObject* rc = PyObject_CallMethodObjArgs(bio, name, b, nullptr);
+    Py_XDECREF(rc);
     std::string e;
-    PyObject* rc = PyErr_Occurred();
+    rc = PyErr_Occurred();
     if (rc)
     {
         e = "Could not write to Py file obj: " + repr(bio);

tests/test_2548.py

@@ -43,5 +43,8 @@ def test_2548():
         expected = 'cycle in structure tree\nstructure tree broken, assume tree is missing\n' * 76
         expected = expected[:-1] # remove trailing newline.
 
+        # 2023-11-14
+        expected = 'cycle in structure tree\nstructure tree broken, assume tree is missing'
+
         assert wt == expected, f'expected:\n    {expected!r}\nwt:\n    {wt!r}\n'
         assert not e

tests/test_2791.py

@@ -0,0 +1,83 @@
+import fitz
+
+import gc
+import os
+import platform
+import sys
+
+
+def merge_pdf(content: bytes, coverpage: bytes):
+   with fitz.Document(stream=coverpage, filetype='pdf') as coverpage_pdf:
+        with fitz.Document(stream=content, filetype='pdf') as content_pdf:
+            coverpage_pdf.insert_pdf(content_pdf)
+            doc = coverpage_pdf.write()
+            return doc
+
+def test_2791():
+    '''
+    Check for memory leaks.
+    '''
+    if os.environ.get('PYMUPDF_RUNNING_ON_VALGRIND') == '1':
+        print(f'test_2791(): not running because PYMUPDF_RUNNING_ON_VALGRIND=1.')
+        return
+    #stat_type = 'tracemalloc'
+    stat_type = 'psutil'
+    if stat_type == 'tracemalloc':
+        import tracemalloc
+        tracemalloc.start(10)
+        def get_stat():
+            current, peak = tracemalloc.get_traced_memory()
+            return current
+    elif stat_type == 'psutil':
+        # We use RSS, as used by mprof.
+        import psutil
+        process = psutil.Process()
+        def get_stat():
+            return process.memory_info().rss
+    else:
+        def get_stat():
+            return 0
+    n = 1000
+    stats = [1] * n
+    for i in range(n):
+        root = os.path.abspath(f'{__file__}/../../tests/resources')  
+        with open(f'{root}/test_2791_content.pdf', 'rb') as content_pdf:
+            with open(f'{root}/test_2791_coverpage.pdf', 'rb') as coverpage_pdf:
+                content = content_pdf.read()
+                coverpage = coverpage_pdf.read()
+                merge_pdf(content, coverpage)
+                sys.stdout.flush()
+
+        gc.collect()
+        stats[i] = get_stat()
+
+    print(f'Memory usage {stat_type=}.')
+    for i, stat in enumerate(stats):
+        sys.stdout.write(f' {stat}')
+        #print(f'    {i}: {stat}')
+    sys.stdout.write('\n')
+    first = stats[2]
+    last = stats[-1]
+    ratio = last / first
+    print(f'{first=} {last=} {ratio=}')
+
+    if platform.system() != 'Linux':
+        # Values from psutil indicate larger memory leaks on non-Linux. Don't
+        # yet know whether this is because rss is measured differently or a
+        # genuine leak is being exposed.
+        print(f'test_2791(): not asserting ratio because not running on Linux.')
+    elif not hasattr(fitz, 'mupdf'):
+        # Classic implementation has unfixed leaks.
+        print(f'test_2791(): not asserting ratio because using classic implementation.')
+    elif [int(x) for x in platform.python_version_tuple()[:2]] < [3, 11]:
+        print(f'test_2791(): not asserting ratio because python version less than 3.11: {platform.python_version()=}.')
+    elif stat_type == 'tracemalloc':
+        # With tracemalloc Before fix to src/extra.i's calls to
+        # PyObject_CallMethodObjArgs, ratio was 4.26; after it was 1.40.
+        assert ratio > 1 and ratio < 1.6
+    elif stat_type == 'psutil':
+        # Prior to fix, ratio was 1.043. After the fix, improved to 1.005, but
+        # varies and sometimes as high as 1.01.
+        assert ratio >= 1 and ratio < 1.015
+    else:
+        pass

tests/test_story.py

@@ -51,7 +51,7 @@ def make_pdf(html, path_out):
                 <p style="page-break-before: always;"></p>
                 <p>After</p>
                 '''),
-            os.path.relpath(f'{__file__}/../../tests/test_2753-out-before.pdf'),
+            os.path.abspath(f'{__file__}/../../tests/test_2753-out-before.pdf'),
             )
 
     doc_after = make_pdf(
@@ -60,7 +60,7 @@ def make_pdf(html, path_out):
                 <p style="page-break-after: always;"></p>
                 <p>After</p>
                 '''),
-            os.path.relpath(f'{__file__}/../../tests/test_2753-out-after.pdf'),
+            os.path.abspath(f'{__file__}/../../tests/test_2753-out-after.pdf'),
             )
 
     assert len(doc_before) == 2

tests/test_toc.py

@@ -100,7 +100,7 @@ def test_2788():
         # Classic implementation does not have fix for this test.
         print(f'Not running test_2788 on classic implementation.')
         return
-    path = os.path.relpath(f'{__file__}/../../tests/resources/test_2788.pdf')        
+    path = os.path.abspath(f'{__file__}/../../tests/resources/test_2788.pdf')        
     document = fitz.open(path)
     toc0 = [[1, 'page2', 2, {'kind': 4, 'xref': 14, 'page': 1, 'to': (100.0, 760.0), 'zoom': 0.0, 'nameddest': 'page.2'}]]
     toc1 = document.get_toc(simple=False)