Handle mismatched HDF5 locking flags

src/ess/reduce/nexus/_nexus_loader.py

@@ -30,6 +30,14 @@ class NoNewDefinitionsType: ...
 NoNewDefinitions = NoNewDefinitionsType()
 
 
+class NoLockingIfNeededType:
+    def __repr__(self) -> str:
+        return "NoLockingIfNeeded"
+
+
+NoLockingIfNeeded = NoLockingIfNeededType()
+
+
 def load_component(
     location: NeXusLocationSpec,
     *,
@@ -93,7 +101,7 @@ def _open_nexus_file(
     file_path: FilePath | NeXusFile | NeXusGroup,
     definitions: Mapping | None | NoNewDefinitionsType = NoNewDefinitions,
     *,
-    locking: bool | None = None,
+    locking: bool | str | None | NoLockingIfNeededType = NoLockingIfNeeded,
 ) -> AbstractContextManager[snx.Group]:
     if isinstance(file_path, getattr(NeXusGroup, '__supertype__', type(None))):
         if (
@@ -106,32 +114,63 @@ def _open_nexus_file(
         return nullcontext(file_path)
 
     try:
-        return _open_nexus_file_from_path(file_path, definitions, locking=locking)
+        return _open_nexus_file_from_path(
+            file_path,
+            definitions,
+            locking=None if locking is NoLockingIfNeeded else locking,
+        )
     except OSError as err:
-        if err.errno == errno.EROFS:
-            # Failed to open because the filesystem is read-only.
-            # (According to https://www.ioplex.com/%7Emiallen/errcmpp.html
-            # this error code is universal.)
-            #
-            # On ESS machines, this happens for network filesystems of data that was
-            # ingested into SciCat, including raw data.
-            # In this case, it is safe to open the file without locking because:
-            # - For raw files, they were written on a separate machine and are synced
-            #   with the one running reduction software. So there cannot be concurrent
-            #   write and read accesses to the same file on the same filesystem.
-            #   The ground truth on the filesystem used by the file writer is protected
-            #   and cannot be corrupted by our reader.
-            # - For processed data, the file was copied to the read-only filesystem.
-            #   So the copy we are opening was not written by HDF5 directly and thus
-            #   locking has no effect anyway.
-            #
-            # When running on user machines, disabling locking can potentially corrupt
-            # files. But the risk is minimal because very few users will have read-only
-            # filesystems and do concurrent reads and writes.
+        if _attempt_to_open_without_locking(err, locking):
             return _open_nexus_file_from_path(file_path, definitions, locking=False)
         raise
 
 
+# On ESS machines, some network filesystems are read-only.
+# E.g., data that was ingested into SciCat, including raw data.
+# HDF5 fails to open such files because it cannot lock the files.
+# In this case, it is safe(*) to open the file without locking because:
+#
+# - For raw files, they were written on a separate machine and are synced
+#   with the one running reduction software. So there cannot be concurrent
+#   write and read accesses to the same file on the same filesystem.
+#   The ground truth on the filesystem used by the file writer is protected
+#   and cannot be corrupted by our reader.
+# - For processed data, the file was copied to the read-only filesystem.
+#   So the copy we are opening was not written by HDF5 directly and thus
+#   locking has no effect anyway.
+#
+# When running on user machines, disabling locking can potentially corrupt
+# files. But the risk is minimal because very few users will have read-only
+# filesystems and do concurrent reads and writes.
+#
+# (*) Files on the read-only filesystem may still change while a file is open for
+# reading if they get updated from the original file. E.g., when reading a file that is
+# currently being written to. This can crash the reader. But our code is anyway not set
+# up to deal with changing files, so the added risk is not significant.
+#
+# See https://github.com/HDFGroup/hdf5/blob/e9ab45f0f4d7240937d5f88055f6c217da80f0d4/doxygen/dox/file-locking.dox
+# about HDF5 file locking.
+def _attempt_to_open_without_locking(
+    err: OSError, locking: bool | str | None | NoLockingIfNeededType
+) -> bool:
+    if locking is not NoLockingIfNeeded:
+        return False  # Respect user's choice.
+    if err.errno == errno.EROFS:
+        # Read-only filesystem.
+        # (According to https://www.ioplex.com/%7Emiallen/errcmpp.html
+        # this error code is universal.)
+        return True
+
+    # HDF5 tracks file locking flags internally within a single process.
+    # If the same file is opened multiple times, we can get a flag mismatch.
+    # We can try opening without locking, maybe this matches the original flags.
+    if "file locking flag values don't match" in err.args[0]:
+        return True
+    if "file locking 'ignore disabled locks' flag values don't match" in err.args[0]:
+        return True
+    return False
+
+
 def _open_nexus_file_from_path(
     file_path: FilePath,
     definitions: Mapping | None | NoNewDefinitionsType,

tests/nexus/nexus_loader_test.py

@@ -13,7 +13,8 @@
 import scippnexus as snx
 
 from ess.reduce import nexus
-from ess.reduce.nexus.types import NeXusLocationSpec
+from ess.reduce.nexus._nexus_loader import NoLockingIfNeeded
+from ess.reduce.nexus.types import FilePath, NeXusLocationSpec
 
 year_zero = sc.datetime('1970-01-01T00:00:00')
 
@@ -625,3 +626,68 @@ def compute_component_position_returns_input_if_no_depends_on() -> None:
     dg = sc.DataGroup(position=sc.vector([1, 2, 3], unit='m'))
     result = nexus.compute_component_position(dg)
     assert result is dg
+
+
+# Some filesystems (e.g., for raw files at ESS) are read-only and
+# h5py cannot open files on these systems with file locks.
+# We cannot reasonably emulate this within Python tests.
+# So the following tests only check the behaviour on a basic level.
+# The tests use the private `_open_nexus_file` directly to focus on what matters.
+#
+# A file may already be open in this or another process.
+# We should still be able to open it
+@pytest.mark.parametrize(
+    "locks",
+    [
+        (False, False),
+        (True, True),
+        (None, None),
+        (NoLockingIfNeeded, NoLockingIfNeeded),
+        # These are ok because the second user adapts to the first:
+        (False, NoLockingIfNeeded),
+        (None, NoLockingIfNeeded),
+        # This would fail on a read-only filesystem because the second case can't adapt:
+        (NoLockingIfNeeded, None),
+    ],
+)
+def test_open_nexus_file_multiple_times(tmp_path: Path, locks: tuple[Any, Any]) -> None:
+    from ess.reduce.nexus._nexus_loader import _open_nexus_file
+
+    path = FilePath(tmp_path / "file.nxs")
+    with snx.File(path, "w"):
+        pass
+    with _open_nexus_file(path, locking=locks[0]) as f1:
+        with _open_nexus_file(path, locking=locks[1]) as f2:
+            assert f1.name == f2.name
+
+
+@pytest.mark.parametrize(
+    "locks",
+    [
+        (True, False),
+        (True, None),
+        (False, True),
+        (False, None),
+        (None, True),
+        (None, False),
+        # On a read-only filesystem, this would work:
+        (NoLockingIfNeeded, False),
+        # This could be supported, but it could cause problems because the first
+        # user expects the file to be locked.
+        (True, NoLockingIfNeeded),
+        # Same as above but with roles reversed:
+        (NoLockingIfNeeded, True),
+    ],
+)
+def test_open_nexus_file_with_mismatched_locking(
+    tmp_path: Path, locks: tuple[Any, Any]
+) -> None:
+    from ess.reduce.nexus._nexus_loader import _open_nexus_file
+
+    path = FilePath(tmp_path / "file.nxs")
+    with snx.File(path, "w"):
+        pass
+
+    with _open_nexus_file(path, locking=locks[0]):
+        with pytest.raises(OSError, match="flag values don't match"):
+            _ = _open_nexus_file(path, locking=locks[1])