diff --git a/xarray/backends/file_manager.py b/xarray/backends/file_manager.py index f7cd4675729..8f36d720de4 100644 --- a/xarray/backends/file_manager.py +++ b/xarray/backends/file_manager.py @@ -238,8 +238,9 @@ 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: - file.close() + if file is None: + return + file.close() def __del__(self) -> None: # If we're the only CachingFileManger referencing a unclosed file, @@ -355,6 +356,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 +364,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. @@ -391,10 +394,13 @@ def acquire_context(self, needs_lock: bool = True) -> Iterator[T_File]: yield self._get_unclosed_file() 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. @@ -417,11 +423,11 @@ def __del__(self) -> None: def __getstate__(self): # file is intentionally omitted: we want to open it again opener = _get_none if self._closed else self._opener - return (opener, self._args, self._mode, self._kwargs) + return (opener, self._args, self._mode, self._lock, self._kwargs) def __setstate__(self, state) -> None: - opener, args, mode, kwargs = state - self.__init__(opener, *args, mode=mode, kwargs=kwargs) # type: ignore[misc] + opener, args, mode, lock, kwargs = state + self.__init__(opener, *args, mode=mode, lock=lock, kwargs=kwargs) # type: ignore[misc] def __repr__(self) -> str: if self._closed: @@ -448,9 +454,16 @@ 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 @@ -464,5 +477,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() diff --git a/xarray/backends/netCDF4_.py b/xarray/backends/netCDF4_.py index bb511f9befc..39dedd139c0 100644 --- a/xarray/backends/netCDF4_.py +++ b/xarray/backends/netCDF4_.py @@ -421,7 +421,7 @@ def __init__( "argument is provided" ) root = manager - manager = DummyFileManager(root) + manager = DummyFileManager(root, lock=NETCDF4_PYTHON_LOCK) self._manager = manager self._group = group @@ -510,17 +510,21 @@ 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)