From 1bab75c2fb16fd584836ca008a6e21b298b99b0f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 19:21:46 +0000 Subject: [PATCH 1/3] lib/vector: Close the sidx file after loading it for update Vect_open_sidx() in update mode left the file handle open when the index was loaded into memory. Vect_save_sidx() then replaced the handle, so the file stayed open until the process ended. On Windows this made removing the map fail with 'Unable to delete file .../sidx'. The new test checks that no handle to the sidx file is left after a map opened with pygrass in 'rw' mode is closed. AI assistance was used to diagnose the failure and to write the fix and the test. --- lib/vector/Vlib/open.c | 6 +++ .../tests/grass_pygrass_vector_sidx_checks.py | 48 +++++++++++++++++++ .../tests/grass_pygrass_vector_sidx_test.py | 47 ++++++++++++++++++ 3 files changed, 101 insertions(+) create mode 100644 python/grass/pygrass/vector/tests/grass_pygrass_vector_sidx_checks.py create mode 100644 python/grass/pygrass/vector/tests/grass_pygrass_vector_sidx_test.py diff --git a/lib/vector/Vlib/open.c b/lib/vector/Vlib/open.c index d6d78fc25d8..ecb7e9f28c9 100644 --- a/lib/vector/Vlib/open.c +++ b/lib/vector/Vlib/open.c @@ -1291,6 +1291,12 @@ int Vect_open_sidx(struct Map_info *Map, int mode) fclose(Plus->spidx_fp.file); return -1; } + if (!Plus->Spidx_file) { + /* the index is fully loaded in memory and the file is + * rewritten by Vect_save_sidx() */ + fclose(Plus->spidx_fp.file); + Plus->spidx_fp.file = NULL; + } } } diff --git a/python/grass/pygrass/vector/tests/grass_pygrass_vector_sidx_checks.py b/python/grass/pygrass/vector/tests/grass_pygrass_vector_sidx_checks.py new file mode 100644 index 00000000000..b2cfba6efa5 --- /dev/null +++ b/python/grass/pygrass/vector/tests/grass_pygrass_vector_sidx_checks.py @@ -0,0 +1,48 @@ +# Copyright (C) 2026 by the GRASS Development Team +# SPDX-License-Identifier: GPL-2.0-or-later + +"""Open a vector map in update mode, close it, and check for a leaked sidx handle + +Run in a GRASS session by ``grass_pygrass_vector_sidx_test.py`` in a +subprocess, because the vector library state is per process. +""" + +import os +import sys + +from grass.pygrass.vector import VectorTopo + + +def sidx_is_open(path): + """Return True if this process still has the file open""" + if sys.platform.startswith("linux"): + # A handle to a replaced file is reported with a " (deleted)" suffix. + return any( + os.path.realpath(f"/proc/self/fd/{fd}").startswith(path) + for fd in os.listdir("/proc/self/fd") + ) + # Windows refuses to delete an open file. + try: + os.remove(path) + except PermissionError: + return True + return False + + +def main(): + """Create a map, reopen it in update mode, and check the sidx file""" + vect = VectorTopo("sidx_map") + vect.open("w") + vect.close() + vect.open("rw") + vect.close() + mapset_path = os.environ["MAPSET_PATH"] + path = os.path.realpath(os.path.join(mapset_path, "vector", "sidx_map", "sidx")) + if sidx_is_open(path): + print(f"The file {path} was left open after closing the map") + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/python/grass/pygrass/vector/tests/grass_pygrass_vector_sidx_test.py b/python/grass/pygrass/vector/tests/grass_pygrass_vector_sidx_test.py new file mode 100644 index 00000000000..ec3c9d98849 --- /dev/null +++ b/python/grass/pygrass/vector/tests/grass_pygrass_vector_sidx_test.py @@ -0,0 +1,47 @@ +# Copyright (C) 2026 by the GRASS Development Team +# SPDX-License-Identifier: GPL-2.0-or-later + +"""Test that closing a vector map opened in update mode releases the sidx file + +An open file handle left behind makes the removal of the map fail on Windows +and leaks a file descriptor on all platforms. +""" + +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_sidx_checks.py") + + +@pytest.fixture(scope="module") +def session(tmp_path_factory): + """Active session in an XY project (scope: module)""" + project = tmp_path_factory.mktemp("sidx") / "project" + gs.create_project(project) + with gs.setup.init(project, env=os.environ.copy()) as session: + yield session + + +def test_update_mode_releases_sidx(session): + """The sidx file is closed after a map opened with "rw" is closed""" + env = session.env.copy() + env["MAPSET_PATH"] = str( + Path(gs.gisenv(env=session.env)["GISDBASE"]) + / gs.gisenv(env=session.env)["LOCATION_NAME"] + / gs.gisenv(env=session.env)["MAPSET"] + ) + result = subprocess.run( + [sys.executable, str(CHECKS)], + env=env, + capture_output=True, + text=True, + check=False, + timeout=120, + ) + assert result.returncode == 0, f"{result.stdout}\n{result.stderr}" From d2d2c6c6d97449632e021de3fbd4005579f62d12 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 19:21:47 +0000 Subject: [PATCH 2/3] grass.pygrass: Compare normalized paths in the get_path doctest The expected path is built with os.path.join(), which uses backslashes on Windows, while get_path() substitutes into a path written with slashes. --- python/grass/pygrass/vector/table.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/grass/pygrass/vector/table.py b/python/grass/pygrass/vector/table.py index ea54af90841..7c92256642f 100644 --- a/python/grass/pygrass/vector/table.py +++ b/python/grass/pygrass/vector/table.py @@ -48,7 +48,7 @@ def get_path(path, vect_name=None): ... "sqlite", ... "sqlite.db", ... ) - >>> new_path.replace("//", "/") == new_path2.replace("//", "/") + >>> os.path.normpath(new_path) == os.path.normpath(new_path2) True >>> path = "$GISDBASE/$LOCATION_NAME/$MAPSET/vector/$MAP/sqlite.db" >>> new_path = get_path(path, "test") @@ -60,7 +60,7 @@ def get_path(path, vect_name=None): ... "test", ... "sqlite.db", ... ) - >>> new_path.replace("//", "/") == new_path2.replace("//", "/") + >>> os.path.normpath(new_path) == os.path.normpath(new_path2) True """ From 27aba9ea94d013507d187ea64d6ceaad3eda320d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 19:23:06 +0000 Subject: [PATCH 3/3] grass.pygrass: Use pathlib to list the open files in the sidx test --- .../pygrass/vector/tests/grass_pygrass_vector_sidx_checks.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/python/grass/pygrass/vector/tests/grass_pygrass_vector_sidx_checks.py b/python/grass/pygrass/vector/tests/grass_pygrass_vector_sidx_checks.py index b2cfba6efa5..b50e033d3db 100644 --- a/python/grass/pygrass/vector/tests/grass_pygrass_vector_sidx_checks.py +++ b/python/grass/pygrass/vector/tests/grass_pygrass_vector_sidx_checks.py @@ -9,6 +9,7 @@ import os import sys +from pathlib import Path from grass.pygrass.vector import VectorTopo @@ -18,8 +19,8 @@ def sidx_is_open(path): if sys.platform.startswith("linux"): # A handle to a replaced file is reported with a " (deleted)" suffix. return any( - os.path.realpath(f"/proc/self/fd/{fd}").startswith(path) - for fd in os.listdir("/proc/self/fd") + os.path.realpath(fd).startswith(path) + for fd in Path("/proc/self/fd").iterdir() ) # Windows refuses to delete an open file. try: