From c37e1b50e11be79e481f55e879c03df33b587ec0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 16:41:36 +0000 Subject: [PATCH 1/4] lib/vector: Return WKB buffers allocated by G_malloc Vect_read_area_to_wkb(), Vect_read_line_to_wkb() and Vect_line_to_wkb() returned the buffer allocated by GEOS. It has to be released with GEOSFree(), but the Python code in pygrass released it with G_free(). That works when all libraries share one C runtime, as on Linux. When GEOS and GRASS use different runtimes, as on Windows, it corrupts the heap and the process crashes (test_pygrass_vector_doctests.py stopped at areas_to_wkb_list()). The WKT functions in the same file already copy the GEOS result into memory from G_store() and free the original. Copy the buffer into memory from G_malloc() and release the GEOS one with GEOSFree(), so G_free() is correct for the caller. Document this. Release the buffer in Line.to_wkb() and Area.to_wkb() in pygrass, which never freed it. Calling Line.to_wkb() 60000 times grew the memory by 4.7 MB before and by nothing after the change. The changes were developed with the help of an AI assistant (Claude). --- lib/vector/Vlib/geos_to_wktb.c | 44 +++++++++++++++++++++---- python/grass/pygrass/vector/geometry.py | 8 +++-- 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/lib/vector/Vlib/geos_to_wktb.c b/lib/vector/Vlib/geos_to_wktb.c index 9aca5f2e69e..b25ab4d6172 100644 --- a/lib/vector/Vlib/geos_to_wktb.c +++ b/lib/vector/Vlib/geos_to_wktb.c @@ -13,11 +13,40 @@ #include #include +#include #include #include #ifdef HAVE_GEOS +/*! + \brief Copy a WKB buffer allocated by GEOS into memory from G_malloc() + + The buffer returned by GEOS has to be released with GEOSFree() of the + library which allocated it. Returning a copy lets the caller use G_free(), + which is not possible with the GEOS buffer when GEOS and GRASS use + different C runtimes (for example on Windows). + + \param geos_wkb buffer allocated by GEOS, released by this function + \param size size of the buffer + + \return pointer to a copy allocated by G_malloc() + \return NULL if geos_wkb is NULL + */ +static unsigned char *copy_geos_wkb(unsigned char *geos_wkb, size_t size) +{ + unsigned char *wkb; + + if (!geos_wkb) + return NULL; + + wkb = G_malloc(size); + memcpy(wkb, geos_wkb, size); + GEOSFree(geos_wkb); + + return wkb; +} + /*! \brief Read vector area and return it as Well Known Binary (WKB) unsigned char array @@ -26,7 +55,8 @@ \param area area id \param size The size of the returned unsigned char array - \return pointer to unsigned char array + \return pointer to unsigned char array allocated by G_malloc(), to be + released with G_free() \return NULL on error */ unsigned char *Vect_read_area_to_wkb(struct Map_info *Map, int area, @@ -56,7 +86,7 @@ unsigned char *Vect_read_area_to_wkb(struct Map_info *Map, int area, GEOSGeom_destroy(geom); - return (wkb); + return copy_geos_wkb(wkb, *size); } /*! @@ -137,7 +167,8 @@ char *Vect_read_area_to_wkt2(struct Map_info *Map, int area, bool trim) \param line The id of the feature to read \param size The size of the returned unsigned char array - \return pointer to unsigned char array + \return pointer to unsigned char array allocated by G_malloc(), to be + released with G_free() \return NULL on error */ unsigned char *Vect_read_line_to_wkb(struct Map_info *Map, @@ -194,7 +225,7 @@ unsigned char *Vect_read_line_to_wkb(struct Map_info *Map, GEOSGeom_destroy(geom); - return (wkb); + return copy_geos_wkb(wkb, *size); } /*! @@ -214,7 +245,8 @@ unsigned char *Vect_read_line_to_wkb(struct Map_info *Map, \param with_z Set to 1 if the feature is 3d, 0 otherwise \param size The size of the returned byte array - \return pointer to string (allocated) + \return pointer to byte array allocated by G_malloc(), to be released + with G_free() \return NULL on error */ unsigned char *Vect_line_to_wkb(const struct line_pnts *points, int type, @@ -244,7 +276,7 @@ unsigned char *Vect_line_to_wkb(const struct line_pnts *points, int type, GEOSGeom_destroy(geom); - return (wkb); + return copy_geos_wkb(wkb, *size); } /*! diff --git a/python/grass/pygrass/vector/geometry.py b/python/grass/pygrass/vector/geometry.py index 63154984b34..23606b206f6 100644 --- a/python/grass/pygrass/vector/geometry.py +++ b/python/grass/pygrass/vector/geometry.py @@ -404,7 +404,9 @@ def to_wkb(self): barray = libvect.Vect_line_to_wkb( self.c_points, self.gtype, not self.is2D, ctypes.byref(size) ) - return ctypes.string_at(barray, size.value) + wkb = ctypes.string_at(barray, size.value) + libgis.G_free(barray) + return wkb class Point(Geo): @@ -1798,7 +1800,9 @@ def to_wkb(self): barray = libvect.Vect_read_area_to_wkb( self.c_mapinfo, self.id, ctypes.byref(size) ) - return ctypes.string_at(barray, size.value) + wkb = ctypes.string_at(barray, size.value) + libgis.G_free(barray) + return wkb @mapinfo_must_be_set def cats(self, cats=None): From 2ef29276391e171ab6672efe44a5360cd0d2d5e1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 16:45:53 +0000 Subject: [PATCH 2/4] grass.pygrass: Test the release of WKB buffers Add pytest tests for the WKB functions of pygrass vector. The checks run in a subprocess in a GRASS session, as the C libraries can be initialized only once per process, and a crash then fails one test. - Line.to_wkb() and Area.to_wkb() release the buffer with G_free(). - The functions returning lists release each buffer with G_free(). - Many calls do not leave memory allocated (glibc only). - All functions return WKB of the expected size when called repeatedly. This is the case which crashes on Windows when a buffer is released with the wrong allocator. Without the change to the vector library and pygrass, the checks for the buffers of Line.to_wkb() and Area.to_wkb() and for the memory fail. The tests were developed with the help of an AI assistant (Claude). --- .../tests/grass_pygrass_vector_wkb_checks.py | 174 ++++++++++++++++++ .../tests/grass_pygrass_vector_wkb_test.py | 66 +++++++ 2 files changed, 240 insertions(+) create mode 100644 python/grass/pygrass/vector/tests/grass_pygrass_vector_wkb_checks.py create mode 100644 python/grass/pygrass/vector/tests/grass_pygrass_vector_wkb_test.py diff --git a/python/grass/pygrass/vector/tests/grass_pygrass_vector_wkb_checks.py b/python/grass/pygrass/vector/tests/grass_pygrass_vector_wkb_checks.py new file mode 100644 index 00000000000..fdc10f4f183 --- /dev/null +++ b/python/grass/pygrass/vector/tests/grass_pygrass_vector_wkb_checks.py @@ -0,0 +1,174 @@ +# Copyright (C) 2026 by the GRASS Development Team +# SPDX-License-Identifier: GPL-2.0-or-later + +"""Checks of the WKB buffers of pygrass vector, run in a GRASS session + +This is run by ``grass_pygrass_vector_wkb_test.py`` in a subprocess with +the name of a check as the argument. A subprocess is used because the C +libraries can be initialized only once per process, and because a crash +(heap corruption) then fails one test instead of ending the test run. + +Exit status 0 means success, ``NOT_SUPPORTED`` means that the check cannot +run on this platform and anything else is a failure. +""" + +import ctypes +import sys + +from grass.pygrass import utils +from grass.pygrass.vector import VectorTopo, geometry +from grass.pygrass.vector.geometry import Area, Line + +NOT_SUPPORTED = 77 +MAP_NAME = "wkb_test_vector" +# Sizes in bytes of the WKB of the areas of the test vector map +AREA_WKB_SIZES = [225, 141, 93, 141] + + +class Mallinfo2(ctypes.Structure): + """struct mallinfo2 of glibc""" + + _fields_ = [ + (name, ctypes.c_size_t) + for name in ( + "arena", + "ordblks", + "smblks", + "hblks", + "hblkhd", + "usmblks", + "fsmblks", + "uordblks", + "fordblks", + "keepcost", + ) + ] + + +def bytes_in_use(): + """Return the number of bytes allocated with malloc and not freed""" + try: + mallinfo2 = ctypes.CDLL(None).mallinfo2 + except (AttributeError, OSError): + print("mallinfo2 of glibc is not available") + sys.exit(NOT_SUPPORTED) + mallinfo2.restype = Mallinfo2 + return mallinfo2().uordblks + + +def open_test_map(): + """Create the test vector map and open it for reading""" + utils.create_test_vector_map(MAP_NAME) + vector = VectorTopo(MAP_NAME) + vector.open("r") + return vector + + +class FreeSpy: + """Record the calls of G_free() of the GRASS library, which are forwarded""" + + def __init__(self): + self.calls = [] + + def __enter__(self): + self._original = geometry.libgis.G_free + + def spy(pointer): + self.calls.append(ctypes.cast(pointer, ctypes.c_void_p).value) + self._original(pointer) + + geometry.libgis.G_free = spy + return self + + def __exit__(self, *args): + geometry.libgis.G_free = self._original + + +def line(): + """Return a line which needs a WKB buffer of 41 bytes""" + return Line([(0, 0), (1, 1)]) + + +def check_line_wkb_is_freed(): + """The buffer of Line.to_wkb() is released with G_free()""" + a_line = line() + with FreeSpy() as spy: + wkb = a_line.to_wkb() + assert len(wkb) == 41, len(wkb) + assert len(spy.calls) == 1, f"G_free() was called {len(spy.calls)} times" + assert spy.calls[0], "G_free() was called with NULL" + + +def check_area_wkb_is_freed(): + """The buffer of Area.to_wkb() is released with G_free()""" + vector = open_test_map() + with FreeSpy() as spy: + wkb = Area(v_id=1, c_mapinfo=vector.c_mapinfo).to_wkb() + vector.close() + assert len(wkb) == AREA_WKB_SIZES[0], len(wkb) + assert len(spy.calls) == 1, f"G_free() was called {len(spy.calls)} times" + assert spy.calls[0], "G_free() was called with NULL" + + +def check_wkb_lists_are_freed(): + """Each buffer of the functions returning a list is released with G_free()""" + vector = open_test_map() + with FreeSpy() as spy: + areas = vector.areas_to_wkb_list() + features = vector.features_to_wkb_list(feature_type="point") + vector.close() + assert [len(wkb) for _, _, wkb in areas] == AREA_WKB_SIZES + assert len(features) == 3, len(features) + expected = len(areas) + len(features) + assert len(spy.calls) == expected, f"{len(spy.calls)} calls, not {expected}" + + +def check_no_memory_growth(): + """Calling the functions many times does not leave memory allocated""" + vector = open_test_map() + area = Area(v_id=1, c_mapinfo=vector.c_mapinfo) + a_line = line() + for _ in range(1000): + a_line.to_wkb() + area.to_wkb() + before = bytes_in_use() + calls = 20000 + for _ in range(calls): + a_line.to_wkb() + area.to_wkb() + growth = bytes_in_use() - before + vector.close() + # One leaked buffer per call would be at least 21 bytes + 225 bytes + assert growth < 200_000, f"{growth} bytes more in use after {calls} calls" + + +def check_all_functions_work_repeatedly(): + """The functions return WKB and survive many calls + + Releasing a buffer with the wrong allocator crashes the process on + Windows, which would fail this check. + """ + vector = open_test_map() + area = Area(v_id=1, c_mapinfo=vector.c_mapinfo) + a_line = line() + for _ in range(500): + wkb = a_line.to_wkb() + assert wkb[0] in (0, 1) + assert len(wkb) == 41 + wkb = area.to_wkb() + assert wkb[0] in (0, 1) + assert len(wkb) == AREA_WKB_SIZES[0] + areas = vector.areas_to_wkb_list() + assert [len(wkb) for _, _, wkb in areas] == AREA_WKB_SIZES + assert len(vector.features_to_wkb_list(feature_type="point")) == 3 + vector.close() + + +def main(): + """Run the check named by the first argument""" + name = sys.argv[1] + globals()[f"check_{name}"]() + + +if __name__ == "__main__": + main() diff --git a/python/grass/pygrass/vector/tests/grass_pygrass_vector_wkb_test.py b/python/grass/pygrass/vector/tests/grass_pygrass_vector_wkb_test.py new file mode 100644 index 00000000000..8a3ade6201d --- /dev/null +++ b/python/grass/pygrass/vector/tests/grass_pygrass_vector_wkb_test.py @@ -0,0 +1,66 @@ +# Copyright (C) 2026 by the GRASS Development Team +# SPDX-License-Identifier: GPL-2.0-or-later + +"""Test the release of the WKB buffers returned by the vector library + +The buffers are allocated with G_malloc() by the vector library, and the +pygrass functions have to release them with G_free(). Releasing a buffer +allocated by GEOS with G_free() crashes on Windows, and not releasing it +leaks memory on all platforms. + +The checks run in a subprocess, see ``grass_pygrass_vector_wkb_checks.py``. +""" + +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +import grass.script as gs + +CHECKS = Path(__file__).with_name("grass_pygrass_vector_wkb_checks.py") +NOT_SUPPORTED = 77 + + +@pytest.fixture(scope="module") +def session(tmp_path_factory): + """Active session in an XY project (scope: module)""" + project = tmp_path_factory.mktemp("wkb") / "project" + gs.create_project(project) + with gs.setup.init(project, env=os.environ.copy()) as session: + yield session + + +def run_check(session, name): + """Run a check in the session and fail the test when the check fails""" + result = subprocess.run( + [sys.executable, str(CHECKS), name], + env=session.env, + capture_output=True, + text=True, + check=False, + timeout=240, + ) + if result.returncode == NOT_SUPPORTED: + pytest.skip(result.stdout.strip()) + assert result.returncode == 0, ( + f"Check {name} ended with status {result.returncode}\n" + f"{result.stdout}\n{result.stderr}" + ) + + +@pytest.mark.parametrize( + "check", + [ + "line_wkb_is_freed", + "area_wkb_is_freed", + "wkb_lists_are_freed", + "no_memory_growth", + "all_functions_work_repeatedly", + ], +) +def test_wkb_buffers(session, check): + """Check that the WKB buffers are released and the functions work""" + run_check(session, check) From aa9bdf8e927c7ea28f61831dc1cbe0aa2cfe3c09 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 17:01:23 +0000 Subject: [PATCH 3/4] grass.pygrass: Use a set for the membership test in the WKB checks The preview rule PLR6201 of ruff, which the Python code quality check applies, asks for a set literal. The change was developed with the help of an AI assistant (Claude). --- .../pygrass/vector/tests/grass_pygrass_vector_wkb_checks.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/grass/pygrass/vector/tests/grass_pygrass_vector_wkb_checks.py b/python/grass/pygrass/vector/tests/grass_pygrass_vector_wkb_checks.py index fdc10f4f183..68bf9077324 100644 --- a/python/grass/pygrass/vector/tests/grass_pygrass_vector_wkb_checks.py +++ b/python/grass/pygrass/vector/tests/grass_pygrass_vector_wkb_checks.py @@ -153,10 +153,10 @@ def check_all_functions_work_repeatedly(): a_line = line() for _ in range(500): wkb = a_line.to_wkb() - assert wkb[0] in (0, 1) + assert wkb[0] in {0, 1} assert len(wkb) == 41 wkb = area.to_wkb() - assert wkb[0] in (0, 1) + assert wkb[0] in {0, 1} assert len(wkb) == AREA_WKB_SIZES[0] areas = vector.areas_to_wkb_list() assert [len(wkb) for _, _, wkb in areas] == AREA_WKB_SIZES From 0603cdaa29d527e576f210d4ba2c6158332aa044 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 17:30:35 +0000 Subject: [PATCH 4/4] grass.pygrass: Skip the memory check of the WKB tests outside Linux ctypes.CDLL(None) raises a TypeError on Windows, so the check failed instead of being skipped. Test for Linux first. The change was developed with the help of an AI assistant (Claude). --- .../pygrass/vector/tests/grass_pygrass_vector_wkb_checks.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/python/grass/pygrass/vector/tests/grass_pygrass_vector_wkb_checks.py b/python/grass/pygrass/vector/tests/grass_pygrass_vector_wkb_checks.py index 68bf9077324..d1cdee47d8a 100644 --- a/python/grass/pygrass/vector/tests/grass_pygrass_vector_wkb_checks.py +++ b/python/grass/pygrass/vector/tests/grass_pygrass_vector_wkb_checks.py @@ -47,6 +47,9 @@ class Mallinfo2(ctypes.Structure): def bytes_in_use(): """Return the number of bytes allocated with malloc and not freed""" + if not sys.platform.startswith("linux"): + print("mallinfo2 of glibc is available only on Linux") + sys.exit(NOT_SUPPORTED) try: mallinfo2 = ctypes.CDLL(None).mallinfo2 except (AttributeError, OSError):