diff --git a/src/mikeio/dfs/_dfs1.py b/src/mikeio/dfs/_dfs1.py index 9756f2972..f37cc126d 100644 --- a/src/mikeio/dfs/_dfs1.py +++ b/src/mikeio/dfs/_dfs1.py @@ -87,11 +87,12 @@ class Dfs1(_Dfs123): def __init__(self, filename: str | Path) -> None: super().__init__(filename) - self._dfs = DfsFileFactory.Dfs1FileOpen(str(filename)) - self._x0: float = self._dfs.SpatialAxis.X0 - self._dx: float = self._dfs.SpatialAxis.Dx - self._nx: int = self._dfs.SpatialAxis.XCount - self._title: str = self._dfs.FileInfo.FileTitle + dfs = DfsFileFactory.Dfs1FileOpen(str(filename)) + self._x0: float = dfs.SpatialAxis.X0 + self._dx: float = dfs.SpatialAxis.Dx + self._nx: int = dfs.SpatialAxis.XCount + self._title: str = dfs.FileInfo.FileTitle + dfs.Close() origin = self._longitude, self._latitude self._geometry = Grid1D( diff --git a/src/mikeio/dfs/_dfs3.py b/src/mikeio/dfs/_dfs3.py index b7e1489f0..5a8633f13 100644 --- a/src/mikeio/dfs/_dfs3.py +++ b/src/mikeio/dfs/_dfs3.py @@ -138,22 +138,21 @@ def __init__(self, filename: str | Path): ) def _read_dfs3_header(self, read_x0y0z0: bool = False) -> None: - self._dfs = DfsFileFactory.Dfs3FileOpen(self._filename) - - self._source = self._dfs + dfs = DfsFileFactory.Dfs3FileOpen(self._filename) if read_x0y0z0: - self._x0 = self._dfs.SpatialAxis.X0 - self._y0 = self._dfs.SpatialAxis.Y0 - self._z0 = self._dfs.SpatialAxis.Z0 - - self._dx = self._dfs.SpatialAxis.Dx - self._dy = self._dfs.SpatialAxis.Dy - self._dz = self._dfs.SpatialAxis.Dz - self._nx = self._dfs.SpatialAxis.XCount - self._ny = self._dfs.SpatialAxis.YCount - self._nz = self._dfs.SpatialAxis.ZCount - self._title = self._dfs.FileInfo.FileTitle + self._x0 = dfs.SpatialAxis.X0 + self._y0 = dfs.SpatialAxis.Y0 + self._z0 = dfs.SpatialAxis.Z0 + + self._dx = dfs.SpatialAxis.Dx + self._dy = dfs.SpatialAxis.Dy + self._dz = dfs.SpatialAxis.Dz + self._nx = dfs.SpatialAxis.XCount + self._ny = dfs.SpatialAxis.YCount + self._nz = dfs.SpatialAxis.ZCount + self._title = dfs.FileInfo.FileTitle + dfs.Close() def read( self, diff --git a/src/mikeio/dfsu/_dfsu.py b/src/mikeio/dfsu/_dfsu.py index 73915f691..61cc105ea 100644 --- a/src/mikeio/dfsu/_dfsu.py +++ b/src/mikeio/dfsu/_dfsu.py @@ -580,6 +580,7 @@ def _dfs_read_item_time_func( ) -> tuple[np.ndarray, pd.Timestamp]: dfs = DfsuFile.Open(self._filename) itemdata = dfs.ReadItemTimeStep(item + 1, step) + dfs.Close() return itemdata.Data, itemdata.Time diff --git a/src/mikeio/generic.py b/src/mikeio/generic.py index d2b73b06c..98439b7ec 100644 --- a/src/mikeio/generic.py +++ b/src/mikeio/generic.py @@ -819,6 +819,8 @@ def avg_time( outdatalist[item][has_value] += d[has_value] steps_list[item][has_value] += 1 + dfs_i.Close() + for item in range(n_items): darray = np.zeros_like(outdatalist[item], dtype=np.float32) if skipna: @@ -945,6 +947,8 @@ def quantile( # TODO should this be static Z coordinates instead? dfs_o.WriteItemTimeStepNext(0.0, znitemdata.Data) + dfs_i.Close() + for item in range(n_items_out): darray = outdatalist[item].astype(np.float32) dfs_o.WriteItemTimeStepNext(0.0, darray) diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 000000000..a4fd656bc --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,27 @@ +"""Shared test fixtures and utilities.""" + +import os +import platform +from pathlib import Path + + +def _count_fds_for_file(filename: str) -> int: + """Count open file descriptors pointing to a specific filename on Linux. + + Uses /proc/self/fd readlinks to identify descriptors matching the target file. + Returns 0 on non-Linux platforms. + """ + if platform.system() != "Linux": + return 0 + + target = str(Path(filename).resolve()) + count = 0 + fd_dir = "/proc/self/fd" + for entry in os.listdir(fd_dir): + try: + link = os.readlink(os.path.join(fd_dir, entry)) + if link == target: + count += 1 + except OSError: + continue + return count diff --git a/tests/test_dfs1.py b/tests/test_dfs1.py index c1db72338..3eff1d863 100644 --- a/tests/test_dfs1.py +++ b/tests/test_dfs1.py @@ -1,9 +1,12 @@ from pathlib import Path +import platform + import numpy as np import pytest import pandas as pd import mikeio +from mikecore.DfsFileFactory import DfsFileFactory def test_filenotexist() -> None: @@ -204,3 +207,43 @@ def test_interp_onepoint_dfs1() -> None: with pytest.raises(AssertionError, match="not possible for Grid1D with one point"): ds[0].interp(x=0) + + +@pytest.mark.skipif( + platform.system() != "Linux", + reason="File descriptor counting via /proc only works on Linux", +) +def test_count_open_fds_sanity_check() -> None: + """Verify _count_fds_for_file detects mikecore file handles.""" + from conftest import _count_fds_for_file + + filename = "tests/testdata/random.dfs1" + assert _count_fds_for_file(filename) == 0 + + dfs = DfsFileFactory.DfsGenericOpen(filename) + try: + assert _count_fds_for_file(filename) >= 1, "opening a file must increase FD count" + finally: + dfs.Close() + + assert _count_fds_for_file(filename) == 0, "closing a file must restore FD count" + + +@pytest.mark.skipif( + platform.system() != "Linux", + reason="File descriptor counting via /proc only works on Linux", +) +def test_dfs1_init_closes_file_handle() -> None: + """Dfs1.__init__ must not leak a file handle. + + Before the fix, Dfs1.__init__ stored the open handle in self._dfs + without closing it, so each live instance held one file descriptor. + """ + from conftest import _count_fds_for_file + + filename = "tests/testdata/random.dfs1" + instances = [] + for _ in range(50): + instances.append(mikeio.Dfs1(filename)) + + assert _count_fds_for_file(filename) == 0 diff --git a/tests/test_dfs2.py b/tests/test_dfs2.py index 2aa3989e4..526a44c75 100644 --- a/tests/test_dfs2.py +++ b/tests/test_dfs2.py @@ -1,5 +1,7 @@ from pathlib import Path import datetime +import platform +from typing import Any from matplotlib import pyplot as plt import numpy as np import pandas as pd @@ -939,3 +941,44 @@ def test_append_mismatch_geometry(tmp_path: Path) -> None: dfs = mikeio.Dfs2(new_filename) with pytest.raises(ValueError, match="geometry"): dfs.append(ds2) + + +@pytest.mark.skipif( + platform.system() != "Linux", + reason="File descriptor counting via /proc only works on Linux", +) +def test_dfs2_init_closes_file_handle() -> None: + """Dfs2.__init__ must not leak a file handle. + + Dfs2.__init__ opens a Dfs2FileOpen handle to read spatial axis info + and must close it before returning. + """ + from conftest import _count_fds_for_file + + filename = "tests/testdata/eq.dfs2" + instances = [] + for _ in range(50): + instances.append(mikeio.Dfs2(filename)) + + assert _count_fds_for_file(filename) == 0 + + +@pytest.mark.skipif( + platform.system() != "Linux", + reason="File descriptor counting via /proc only works on Linux", +) +def test_dfs2_read_closes_file_handle() -> None: + """Dfs2.read() must not leak file handles. + + Each read() opens a Dfs2FileOpen handle via _open(); + it must be closed before returning. + """ + from conftest import _count_fds_for_file + + filename = "tests/testdata/eq.dfs2" + results = [] + for _ in range(50): + dfs = mikeio.Dfs2(filename) + results.append(dfs.read()) + + assert _count_fds_for_file(filename) == 0 diff --git a/tests/test_dfs3.py b/tests/test_dfs3.py index 86065b4f2..d0ed2fae8 100644 --- a/tests/test_dfs3.py +++ b/tests/test_dfs3.py @@ -1,4 +1,6 @@ from pathlib import Path +import platform + import pytest import numpy as np @@ -287,6 +289,26 @@ def test_append_dfs3(tmp_path: Path) -> None: dfs.append(ds2) +@pytest.mark.skipif( + platform.system() != "Linux", + reason="File descriptor counting via /proc only works on Linux", +) +def test_dfs3_init_closes_file_handle() -> None: + """Dfs3._read_dfs3_header must not leak a file handle. + + Before the fix, _read_dfs3_header stored the open handle in + self._dfs without closing it. + """ + from conftest import _count_fds_for_file + + filename = "tests/testdata/Grid1.dfs3" + instances = [] + for _ in range(50): + instances.append(mikeio.Dfs3(filename)) + + assert _count_fds_for_file(filename) == 0 + + def test_read_with_title() -> None: sourcefilename = "tests/testdata/single_layer.dfs3" dfs = mikeio.Dfs3(sourcefilename) diff --git a/tests/test_dfsu2dh.py b/tests/test_dfsu2dh.py index 1df5649ad..d65db727d 100644 --- a/tests/test_dfsu2dh.py +++ b/tests/test_dfsu2dh.py @@ -1,3 +1,4 @@ +import platform import shutil from pathlib import Path @@ -1037,6 +1038,26 @@ def test_dfsu_to_xarray_has_element_coordinates() -> None: assert xr_da.z.values[example_quad_element] == approx(example_quad_coordinates[2]) +@pytest.mark.skipif( + platform.system() != "Linux", + reason="File descriptor counting via /proc only works on Linux", +) +def test_dfsu_read_closes_file_handle() -> None: + """Dfsu read must not leak file handles. + + Each read() opens a DfsuFile; it must be closed before returning. + """ + from conftest import _count_fds_for_file + + filename = "tests/testdata/HD2D.dfsu" + results = [] + for _ in range(50): + dfs = mikeio.open(filename) + results.append(dfs.read()) + + assert _count_fds_for_file(filename) == 0 + + def test_write_dfsu_with_title(tmp_path: Path) -> None: """Test writing a dfsu file with a custom title and reading it back.""" sourcefilename = "tests/testdata/HD2D.dfsu"