Fix memory leak in str() of fmpz_mpoly, fmpq_mpoly, fmpz_mod_mpoly, nmod_mpoly

README.md

@@ -170,9 +170,15 @@ Contributors (0.10.0):
 - Vitaly Magerya (VM)
 - Oscar Benjamin (OB)
 - Ilia Gradina (IG)
+- Uri K.H. (UKH)
 
 Changes (0.10.0):
 
+- [gh-435](https://github.com/flintlib/python-flint/issues/435),
+  Fix a memory leak in `str()` (and so in `repr()`, which uses it unless
+  `flint.ctx.pretty` is `False`) of `fmpz_mpoly`, `fmpq_mpoly`,
+  `fmpz_mod_mpoly` and `nmod_mpoly`: the string returned by FLINT was
+  copied into a Python `bytes` object but never freed. (UKH)
 - [gh-433](https://github.com/flintlib/python-flint/pull/433),
   Define the input type aliases in the type stubs (such as `iarb` and
   `ifmpz`) after the classes they reference, so that the ty type checker

src/flint/test/test_all.py

@@ -3966,6 +3966,46 @@ def test_fmpq_mpoly_zcontent_zpoly():
     assert f.zpoly() == 0
 
 
+def _peak_rss_mb() -> float:
+    """Peak resident set size of this process in MB (``resource`` is Unix only)."""
+    import resource
+    peak = resource.getrusage(resource.RUSAGE_SELF).ru_maxrss
+    return peak / (2**20 if platform.system() == "Darwin" else 1024)
+
+
+def test_mpoly_str_does_not_leak():
+    # gh-435: str() of the multivariate types copied the C string returned by
+    # *_mpoly_get_str_pretty into a bytes object and never freed the original.
+    # repr() goes through str() too (flint.ctx.pretty is True by default).
+    try:
+        import resource  # noqa: F401
+    except ImportError:
+        return  # no way to measure memory portably (Windows)
+
+    names = [f"x{i}" for i in range(8)]
+    contexts = [
+        flint.fmpz_mpoly_ctx.get(names),
+        flint.fmpq_mpoly_ctx.get(names),
+        flint.fmpz_mod_mpoly_ctx.get(names, 2**127 - 1),
+        flint.nmod_mpoly_ctx.get(names, 1000003),
+    ]
+    for ctx in contexts:
+        # (sum of generators + 5)^4: 495 terms, so the string is ~10 KB and an
+        # unfreed copy costs ~25 KB per call (4000 calls: ~100 MB unfixed).
+        p = (sum(ctx.gens()) + 5) ** 4
+        assert len(p) == 495
+        expected = str(p)
+        assert expected == p.str() == repr(p)
+        for _ in range(200):  # warm up allocator pools before measuring
+            p.str()
+        before = _peak_rss_mb()
+        for _ in range(4000):
+            p.str()
+        assert str(p) == expected
+        grown = _peak_rss_mb() - before
+        assert grown < 20, f"{type(p).__name__}.str() leaked {grown:.0f} MB in 4000 calls"
+
+
 def _all_mpoly_vecs():
     return [(flint.fmpz_mpoly_ctx, flint.fmpz_mpoly_vec), (flint.fmpq_mpoly_ctx, flint.fmpq_mpoly_vec)]
 

src/flint/types/fmpq_mpoly.pyx

@@ -10,6 +10,7 @@ from flint.flint_base.flint_base cimport (
 )
 
 from flint.utils.typecheck cimport typecheck
+from flint.flintlib.functions.flint cimport flint_free
 from flint.utils.flint_exceptions import DomainError, IncompatibleContextError
 
 from flint.types.fmpq cimport fmpq, any_as_fmpq
@@ -760,9 +761,13 @@ cdef class fmpq_mpoly(flint_mpoly):
         return f"{self.ctx}.from_dict({self.to_dict()})"
 
     def str(self):
-        cdef bytes s = <bytes> fmpq_mpoly_get_str_pretty(self.val, self.ctx.c_names, self.ctx.val)
+        cdef char * cs = fmpq_mpoly_get_str_pretty(self.val, self.ctx.c_names, self.ctx.val)
+        try:
+            s = (<bytes> cs).decode()
+        finally:
+            flint_free(cs)  # FLINT malloc'ed the string; the bytes cast copies it
         # res = s.decode().replace("+", " + ").replace("-", " - ")
-        res = s.decode()
+        res = s
         if res.startswith("- "):
             res = "-" + res[3:]
         return res

src/flint/types/fmpz_mod_mpoly.pyx

@@ -10,6 +10,7 @@ from flint.flint_base.flint_base cimport (
 )
 
 from flint.utils.typecheck cimport typecheck
+from flint.flintlib.functions.flint cimport flint_free
 from flint.utils.flint_exceptions import DomainError, IncompatibleContextError
 
 from flint.types.fmpz cimport any_as_fmpz, fmpz
@@ -746,8 +747,12 @@ cdef class fmpz_mod_mpoly(flint_mpoly):
         return f"{self.ctx}.from_dict({self.to_dict()})"
 
     def str(self):
-        cdef bytes s = <bytes> fmpz_mod_mpoly_get_str_pretty(self.val, self.ctx.c_names, self.ctx.val)
-        res = s.decode().replace("+", " + ").replace("-", " - ")
+        cdef char * cs = fmpz_mod_mpoly_get_str_pretty(self.val, self.ctx.c_names, self.ctx.val)
+        try:
+            s = (<bytes> cs).decode()
+        finally:
+            flint_free(cs)  # FLINT malloc'ed the string; the bytes cast copies it
+        res = s.replace("+", " + ").replace("-", " - ")
         if res.startswith(" - "):
             res = "-" + res[3:]
         return res

src/flint/types/fmpz_mpoly.pyx

@@ -10,6 +10,7 @@ from flint.flint_base.flint_base cimport (
 )
 
 from flint.utils.typecheck cimport typecheck
+from flint.flintlib.functions.flint cimport flint_free
 from flint.utils.flint_exceptions import DomainError, IncompatibleContextError
 
 from flint.types.fmpz cimport any_as_fmpz, fmpz
@@ -734,8 +735,12 @@ cdef class fmpz_mpoly(flint_mpoly):
         return f"{self.ctx}.from_dict({self.to_dict()})"
 
     def str(self):
-        cdef bytes s = <bytes> fmpz_mpoly_get_str_pretty(self.val, self.ctx.c_names, self.ctx.val)
-        res = s.decode().replace("+", " + ").replace("-", " - ")
+        cdef char * cs = fmpz_mpoly_get_str_pretty(self.val, self.ctx.c_names, self.ctx.val)
+        try:
+            s = (<bytes> cs).decode()
+        finally:
+            flint_free(cs)  # FLINT malloc'ed the string; the bytes cast copies it
+        res = s.replace("+", " + ").replace("-", " - ")
         if res.startswith(" - "):
             res = "-" + res[3:]
         return res

src/flint/types/nmod_mpoly.pyx

@@ -8,6 +8,7 @@ from flint.flint_base.flint_base cimport (
 )
 
 from flint.utils.typecheck cimport typecheck
+from flint.flintlib.functions.flint cimport flint_free
 from flint.utils.flint_exceptions import DomainError, IncompatibleContextError
 
 from flint.types.fmpz cimport fmpz
@@ -720,8 +721,12 @@ cdef class nmod_mpoly(flint_mpoly):
         return f"{self.ctx}.from_dict({self.to_dict()})"
 
     def str(self):
-        cdef bytes s = <bytes> nmod_mpoly_get_str_pretty(self.val, self.ctx.c_names, self.ctx.val)
-        res = s.decode().replace("+", " + ").replace("-", " - ")
+        cdef char * cs = nmod_mpoly_get_str_pretty(self.val, self.ctx.c_names, self.ctx.val)
+        try:
+            s = (<bytes> cs).decode()
+        finally:
+            flint_free(cs)  # FLINT malloc'ed the string; the bytes cast copies it
+        res = s.replace("+", " + ").replace("-", " - ")
         if res.startswith(" - "):
             res = "-" + res[3:]
         return res