From 8429f3143042ba701a466c54301ddb6922491259 Mon Sep 17 00:00:00 2001 From: David Bold Date: Wed, 24 Sep 2025 21:06:49 +0200 Subject: [PATCH 01/10] Ensure netcdf4 is locked while closing --- xarray/backends/file_manager.py | 7 ++++++- xarray/backends/locks.py | 2 ++ xarray/backends/netCDF4_.py | 3 +-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/xarray/backends/file_manager.py b/xarray/backends/file_manager.py index f7cd4675729..ead000b6dbd 100644 --- a/xarray/backends/file_manager.py +++ b/xarray/backends/file_manager.py @@ -8,7 +8,7 @@ from contextlib import AbstractContextManager, contextmanager from typing import Any, Generic, Literal, TypeVar, cast -from xarray.backends.locks import acquire +from xarray.backends.locks import acquire, NETCDF4_PYTHON_LOCK from xarray.backends.lru_cache import LRUCache from xarray.core import utils from xarray.core.options import OPTIONS @@ -239,6 +239,11 @@ def close(self, needs_lock: bool = True) -> None: default = None file = self._cache.pop(self._key, default) if file is not None: + # Check for string so we do not have to import it + if str(type(file)) == "": + with NETCDF4_PYTHON_LOCK: + file.close() + return file.close() def __del__(self) -> None: diff --git a/xarray/backends/locks.py b/xarray/backends/locks.py index 784443544ee..4d03511aecf 100644 --- a/xarray/backends/locks.py +++ b/xarray/backends/locks.py @@ -281,3 +281,5 @@ def ensure_lock(lock: Lock | None | Literal[False]) -> Lock: if lock is None or lock is False: return DummyLock() return lock + +NETCDF4_PYTHON_LOCK = combine_locks([NETCDFC_LOCK, HDF5_LOCK]) diff --git a/xarray/backends/netCDF4_.py b/xarray/backends/netCDF4_.py index 8d4ca6441c9..97d6be41122 100644 --- a/xarray/backends/netCDF4_.py +++ b/xarray/backends/netCDF4_.py @@ -32,6 +32,7 @@ from xarray.backends.locks import ( HDF5_LOCK, NETCDFC_LOCK, + NETCDF4_PYTHON_LOCK, combine_locks, ensure_lock, get_write_lock, @@ -66,8 +67,6 @@ # string used by netCDF4. _endian_lookup = {"=": "native", ">": "big", "<": "little", "|": "native"} -NETCDF4_PYTHON_LOCK = combine_locks([NETCDFC_LOCK, HDF5_LOCK]) - class BaseNetCDF4Array(BackendArray): __slots__ = ("datastore", "dtype", "shape", "variable_name") From 70c659c4cd5947590dd2dc5861934eb5366ca43b Mon Sep 17 00:00:00 2001 From: David Bold Date: Fri, 26 Sep 2025 11:02:34 +0200 Subject: [PATCH 02/10] Add locking to DummyFileManager --- xarray/backends/file_manager.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/xarray/backends/file_manager.py b/xarray/backends/file_manager.py index ead000b6dbd..209d814681c 100644 --- a/xarray/backends/file_manager.py +++ b/xarray/backends/file_manager.py @@ -453,9 +453,13 @@ def _remove_del_methods(): class DummyFileManager(FileManager[T_File]): """FileManager that simply wraps an open file in the FileManager interface.""" - def __init__(self, value: T_File, *, close: Callable[[], None] | None = None): + def __init__( + self, value: T_File, *, close: Callable[[], None] | None = None + lock: Lock | None | Literal[False] = None, + ): if close is None: close = value.close + self._lock = lock self._value = value self._close = close @@ -469,5 +473,8 @@ def acquire_context(self, needs_lock: bool = True) -> Iterator[T_File]: yield self._value def close(self, needs_lock: bool = True) -> None: - del needs_lock # unused - self._close() + if needs_lock and self._lock: + with self._lock: + self._close() + else: + self._close() From 2d63c3747acd829967bd99788634c675c3cb68bd Mon Sep 17 00:00:00 2001 From: David Bold Date: Thu, 25 Sep 2025 07:46:17 +0200 Subject: [PATCH 03/10] Remove special handling for netCDF4 in CachingFileManager --- xarray/backends/file_manager.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/xarray/backends/file_manager.py b/xarray/backends/file_manager.py index 209d814681c..687fae74924 100644 --- a/xarray/backends/file_manager.py +++ b/xarray/backends/file_manager.py @@ -239,11 +239,6 @@ def close(self, needs_lock: bool = True) -> None: default = None file = self._cache.pop(self._key, default) if file is not None: - # Check for string so we do not have to import it - if str(type(file)) == "": - with NETCDF4_PYTHON_LOCK: - file.close() - return file.close() def __del__(self) -> None: From e7c84a724df842b8d5dd62d6b7f0a67cf7fa56ce Mon Sep 17 00:00:00 2001 From: David Bold Date: Fri, 26 Sep 2025 11:06:28 +0200 Subject: [PATCH 04/10] Provide lock for FileManagers --- xarray/backends/file_manager.py | 2 ++ xarray/backends/netCDF4_.py | 9 +++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/xarray/backends/file_manager.py b/xarray/backends/file_manager.py index 687fae74924..f1d443d979d 100644 --- a/xarray/backends/file_manager.py +++ b/xarray/backends/file_manager.py @@ -355,6 +355,7 @@ def __init__( opener: Callable[..., T_File], *args: Any, mode: Any = _OMIT_MODE, + lock: Lock | None | Literal[False] = None, kwargs: Mapping[str, Any] | None = None, ): kwargs = {} if kwargs is None else dict(kwargs) @@ -362,6 +363,7 @@ def __init__( self._args = args self._mode = "a" if mode == "w" else mode self._kwargs = kwargs + self._lock = lock # Note: No need for locking with PickleableFileManager, because all # opening of files happens in the constructor. diff --git a/xarray/backends/netCDF4_.py b/xarray/backends/netCDF4_.py index 97d6be41122..a762a847c01 100644 --- a/xarray/backends/netCDF4_.py +++ b/xarray/backends/netCDF4_.py @@ -419,7 +419,7 @@ def __init__( "argument is provided" ) root = manager - manager = DummyFileManager(root) + manager = DummyFileManager(root, lock=NETCDF4_PYTHON_LOCK) self._manager = manager self._group = group @@ -508,17 +508,18 @@ def open( "", mode=mode, memory=memory, **kwargs ) close = _CloseWithCopy(filename, nc4_dataset) - manager = DummyFileManager(nc4_dataset, close=close) + manager = DummyFileManager(nc4_dataset, close=close, lock=lock) elif isinstance(filename, bytes | memoryview): assert mode == "r" kwargs["memory"] = filename manager = PickleableFileManager( - netCDF4.Dataset, "", mode=mode, kwargs=kwargs + netCDF4.Dataset, "", mode=mode, kwargs=kwargs, + lock=lock ) else: manager = CachingFileManager( - netCDF4.Dataset, filename, mode=mode, kwargs=kwargs + netCDF4.Dataset, filename, mode=mode, kwargs=kwargs, lock=lock ) return cls(manager, group=group, mode=mode, lock=lock, autoclose=autoclose) From b414a72670b6331805c6d36254a99d7d3d088b30 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 26 Sep 2025 09:07:07 +0000 Subject: [PATCH 05/10] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- xarray/backends/locks.py | 1 + xarray/backends/netCDF4_.py | 10 ++++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/xarray/backends/locks.py b/xarray/backends/locks.py index 4d03511aecf..e2db5e931d3 100644 --- a/xarray/backends/locks.py +++ b/xarray/backends/locks.py @@ -282,4 +282,5 @@ def ensure_lock(lock: Lock | None | Literal[False]) -> Lock: return DummyLock() return lock + NETCDF4_PYTHON_LOCK = combine_locks([NETCDFC_LOCK, HDF5_LOCK]) diff --git a/xarray/backends/netCDF4_.py b/xarray/backends/netCDF4_.py index a762a847c01..277e08fb196 100644 --- a/xarray/backends/netCDF4_.py +++ b/xarray/backends/netCDF4_.py @@ -30,9 +30,8 @@ PickleableFileManager, ) from xarray.backends.locks import ( - HDF5_LOCK, - NETCDFC_LOCK, NETCDF4_PYTHON_LOCK, + NETCDFC_LOCK, combine_locks, ensure_lock, get_write_lock, @@ -514,8 +513,11 @@ def open( assert mode == "r" kwargs["memory"] = filename manager = PickleableFileManager( - netCDF4.Dataset, "", mode=mode, kwargs=kwargs, - lock=lock + netCDF4.Dataset, + "", + mode=mode, + kwargs=kwargs, + lock=lock, ) else: manager = CachingFileManager( From 74d6141533528ef208b6a7e0d7e0af48c21f6782 Mon Sep 17 00:00:00 2001 From: David Bold Date: Fri, 26 Sep 2025 19:15:40 +0200 Subject: [PATCH 06/10] Add missing comma --- xarray/backends/file_manager.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/xarray/backends/file_manager.py b/xarray/backends/file_manager.py index f1d443d979d..7c55cb2071f 100644 --- a/xarray/backends/file_manager.py +++ b/xarray/backends/file_manager.py @@ -451,7 +451,10 @@ class DummyFileManager(FileManager[T_File]): """FileManager that simply wraps an open file in the FileManager interface.""" def __init__( - self, value: T_File, *, close: Callable[[], None] | None = None + self, + value: T_File, + *, + close: Callable[[], None] | None = None, lock: Lock | None | Literal[False] = None, ): if close is None: From 3b7ca7a368854e50d977c8c0f9463ff2c2c061a9 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 26 Sep 2025 17:16:20 +0000 Subject: [PATCH 07/10] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- xarray/backends/file_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xarray/backends/file_manager.py b/xarray/backends/file_manager.py index 7c55cb2071f..d22ff09b9c5 100644 --- a/xarray/backends/file_manager.py +++ b/xarray/backends/file_manager.py @@ -8,7 +8,7 @@ from contextlib import AbstractContextManager, contextmanager from typing import Any, Generic, Literal, TypeVar, cast -from xarray.backends.locks import acquire, NETCDF4_PYTHON_LOCK +from xarray.backends.locks import acquire from xarray.backends.lru_cache import LRUCache from xarray.core import utils from xarray.core.options import OPTIONS From e79781b1b7fc00bcdedd49b3db452f42fb303c55 Mon Sep 17 00:00:00 2001 From: David Bold Date: Fri, 26 Sep 2025 19:52:44 +0200 Subject: [PATCH 08/10] Use locks for closing files --- xarray/backends/file_manager.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/xarray/backends/file_manager.py b/xarray/backends/file_manager.py index d22ff09b9c5..746b97da5da 100644 --- a/xarray/backends/file_manager.py +++ b/xarray/backends/file_manager.py @@ -238,7 +238,10 @@ def close(self, needs_lock: bool = True) -> None: with self._optional_lock(needs_lock): default = None file = self._cache.pop(self._key, default) - if file is not None: + if needs_lock and self._lock: + with self._lock: + file.close() + else: file.close() def __del__(self) -> None: @@ -396,7 +399,11 @@ def close(self, needs_lock: bool = True) -> None: del needs_lock # unused if not self._closed: file = self._get_unclosed_file() - file.close() + if needs_lock and self._lock: + with self._lock: + file.close() + else: + file.close() self._file = None # Remove all references to opener arguments, so they can be garbage # collected. From 1d9fe51ac61bdb96e12942bf9a5af870c11cf7a4 Mon Sep 17 00:00:00 2001 From: David Bold Date: Fri, 26 Sep 2025 19:54:53 +0200 Subject: [PATCH 09/10] Move back NETCDF4_PYTHON_LOCK --- xarray/backends/locks.py | 3 --- xarray/backends/netCDF4_.py | 3 ++- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/xarray/backends/locks.py b/xarray/backends/locks.py index e2db5e931d3..784443544ee 100644 --- a/xarray/backends/locks.py +++ b/xarray/backends/locks.py @@ -281,6 +281,3 @@ def ensure_lock(lock: Lock | None | Literal[False]) -> Lock: if lock is None or lock is False: return DummyLock() return lock - - -NETCDF4_PYTHON_LOCK = combine_locks([NETCDFC_LOCK, HDF5_LOCK]) diff --git a/xarray/backends/netCDF4_.py b/xarray/backends/netCDF4_.py index 277e08fb196..bd20769f1cc 100644 --- a/xarray/backends/netCDF4_.py +++ b/xarray/backends/netCDF4_.py @@ -30,7 +30,7 @@ PickleableFileManager, ) from xarray.backends.locks import ( - NETCDF4_PYTHON_LOCK, + HDF5_LOCK, NETCDFC_LOCK, combine_locks, ensure_lock, @@ -66,6 +66,7 @@ # string used by netCDF4. _endian_lookup = {"=": "native", ">": "big", "<": "little", "|": "native"} +NETCDF4_PYTHON_LOCK = combine_locks([NETCDFC_LOCK, HDF5_LOCK]) class BaseNetCDF4Array(BackendArray): __slots__ = ("datastore", "dtype", "shape", "variable_name") From c94b7683e72d89e79cfb85158b7a3c3e099c0fd9 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 26 Sep 2025 17:55:18 +0000 Subject: [PATCH 10/10] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- xarray/backends/netCDF4_.py | 1 + 1 file changed, 1 insertion(+) diff --git a/xarray/backends/netCDF4_.py b/xarray/backends/netCDF4_.py index bd20769f1cc..2423e399a31 100644 --- a/xarray/backends/netCDF4_.py +++ b/xarray/backends/netCDF4_.py @@ -68,6 +68,7 @@ NETCDF4_PYTHON_LOCK = combine_locks([NETCDFC_LOCK, HDF5_LOCK]) + class BaseNetCDF4Array(BackendArray): __slots__ = ("datastore", "dtype", "shape", "variable_name")