Repository: h5py/h5py (python)

Issue:
External link files are opened with file locking enabled even if main file is opened with locking disabled
<details><summary>Summary of the h5py configuration</summary>

```
h5py    3.11.0  # current master
HDF5    1.14.4
Python  3.11.2 (main, Mar 13 2023, 12:18:29) [GCC 12.2.0]
sys.platform    linux
sys.maxsize     9223372036854775807
numpy   2.0.0rc2
cython (built with) 3.0.10
numpy (built against) 2.0.0rc2
HDF5 (built against) 1.14.4
```

</details>

We ran into an issue when accessing external links with HDF5's file locking disabled.
The target file of the external link is always opened with file locking enabled even if the main file was opened with file locking disabled, while one would expect to use consistent file access properties.

This has been first raised to the HDFGroup (see: https://forum.hdfgroup.org/t/external-link-and-file-locking-disabled-issue/12012) and fixed in libhdf5 v1.14.4.

Since `h5py` uses it's own default link access property list `h5py.base.default_lapl` which defines an external link file access property list (`set_elink_fapl`), libhdf5 default is ignored:

https://github.com/h5py/h5py/blob/902e32c3fdc06f1e0feaa29717df83e92953dc2a/h5py/_hl/base.py#L129-L135


<details><summary>Script to reproduce the issue</summary>

```python
import multiprocessing
import time
import h5py

def create_files():
    with h5py.File("linked.h5", "w") as h5f:
        h5f["data"] = (1, 2, 3)

    with h5py.File("main.h5", "w") as h5f:
        h5f["data"] = (3, 2, 1)
        h5f["link"] = h5py.ExternalLink("linked.h5", "/data")


def read():
    with h5py.File("main.h5", "r", locking=False) as h5f:
        print("data:", h5f["/data"][()])
        print("link:", h5f["/link"][()])


if __name__ == "__main__":
    print("* Create files")
    create_files()
    print("* Open files in append mode")
    main = h5py.File("main.h5", "a")
    linked = h5py.File("linked.h5", "a")
    print("* Read from subprocess")
    context = multiprocessing.get_context("spawn")
    process = context.Process(target=read)
    process.start()
    time.sleep(1)
```

</details>

Also, while looking at this issue, I found an issue in the values of `H5F_close_degree_t` enum which differs from libhdf5 ones:

- libhdf5's [H5Fpublic.h](https://github.com/HDFGroup/hdf5/blob/d3f5e07b1eea230c7ab340ac9fe24dffa8406355/src/H5Fpublic.h#L114-L119) and [version from 23 years ago](https://github.com/HDFGroup/hdf5/blob/d28fd08f4a9593bd8bb51ebc17b8b6555b3885cd/src/H5Fpublic.h#L72-L85):

```
typedef enum H5F_close_degree_t {
    H5F_CLOSE_DEFAULT = 0, /**< Use the degree pre-defined by underlying VFD */
    H5F_CLOSE_WEAK    = 1, /**< File closes only after all opened objects are closed */
    H5F_CLOSE_SEMI    = 2, /**< If no opened objects, file is closed; otherwise, file close fails */
    H5F_CLOSE_STRONG  = 3  /**< If there are opened objects, close them first, then close file */
} H5F_close_degree_t;
```

- h5py: https://github.com/h5py/h5py/blob/902e32c3fdc06f1e0feaa29717df83e92953dc2a/h5py/api_types_hdf5.pxd#L117-L121


I'll prepare a PR to fix the enum and to propose ways to solve this issue for discussion.

Pull request description:
Fix H5F_close_degree_t and update default link access properties
<!--
Thanks for contributing to h5py!

Before opening a pull request, please:

- Run simple static checks with `tox -e pre-commit`
- Run the tests with e.g. `tox -e py37-test-deps`
- If your change is visible to someone using or building h5py, add a release
  note in the news/ folder.

For more information, see the contribution guide:
http://docs.h5py.org/en/stable/contributing.html#how-to-get-your-code-into-h5py

-->

This PR proposes 2 fixes (I can split it in 2 PRs if you prefer):
- Update the values of the `H5F_close_degree_t` enum to be the same as in [H5Fpublic.h](https://github.com/HDFGroup/hdf5/blob/d3f5e07b1eea230c7ab340ac9fe24dffa8406355/src/H5Fpublic.h#L114-L119)
- Update the default "link access property list" used by `h5py` to be the default one and add a test of external link access with file locking disabled which is impaired by the fact that `h5py` overrides the default "link access property list".

I'm not sure what the side effect of the default "link access property list" change can be!

The fact that the following code has been using the wrong value for `CLOSE_STRONG` and was actually using `CLOSE_SEMI` for over a decade without notice makes me think it should have little impact:

https://github.com/h5py/h5py/blob/902e32c3fdc06f1e0feaa29717df83e92953dc2a/h5py/_hl/base.py#L129-L135

Also, looking at the hdf5 documentation, the "link access property list" is actually not always used:

- [H5Lcreate_external documentation](https://docs.hdfgroup.org/hdf5/v1_14/v1_14_4/group___h5_l.html#ga15dfaeb9b1c0b3136533cb97ee45e683):

  > lapl_id is the link access property list used in traversing the new link. Note that an external file opened by the traversal of an external link is always opened with the weak file close degree property setting, H5F_CLOSE_WEAK (see H5Pset_fclose_degree()); any file close degree property setting in lapl_id is ignored.
  > ...
  > Attention
  >     A file close degree property setting (H5Pset_fclose_degree()) in the external link file access property list or in the external link callback function will be ignored. A file opened by means of traversing an external link is always opened with the weak file close degree property setting, H5F_CLOSE_WEAK .

- [H5Oget_info_by_idx*() H5Oget_info_by_name*()](https://docs.hdfgroup.org/hdf5/v1_14/v1_14_4/group___h5_o.html#title23)

  > The link access property list, lapl_id, is not currently used; it should be passed in as H5P_DEFAULT

So this PR proposes to let libhdf5 use its default (and so disable file locking when accessing external links with libhdf5 >=v1.14.4).


There is a more conservative way to fix the external link+file locking disabled issue (#2431).
It is to copy the file locking properties from the current file into the "link access property list":

```python
class Group(HLObject, MutableMappingHDF5):
   ...
    @property
    def _lapl(self):
        """ Fetch the link access property list appropriate for this object
        """
        lapl = h5p.create(h5p.LINK_ACCESS)
        fapl = h5p.create(h5p.FILE_ACCESS)
        fapl.set_fclose_degree(h5f.CLOSE_STRONG)
        group_fapl = self.file.id.get_access_plist()
        use_file_locking, ignore_when_disabled = group_fapl.get_file_locking()
        fapl.set_file_locking(use_file_locking, ignore_when_disabled)
        lapl.set_elink_fapl(fapl)
        return lapl
```

closes #2431




Patch:
diff --git a/docs_api/h5f.rst b/docs_api/h5f.rst
index 42975a4b..5ccd0d57 100644
--- a/docs_api/h5f.rst
+++ b/docs_api/h5f.rst
@@ -54,10 +54,10 @@ File access flags
 File close strength
 ~~~~~~~~~~~~~~~~~~~
 
+.. data:: CLOSE_DEFAULT
 .. data:: CLOSE_WEAK
 .. data:: CLOSE_SEMI
 .. data:: CLOSE_STRONG
-.. data:: CLOSE_DEFAULT
 
 .. _ref.h5f.SCOPE:
 
diff --git a/h5py/_hl/base.py b/h5py/_hl/base.py
index 9d261c90..93574876 100644
--- a/h5py/_hl/base.py
+++ b/h5py/_hl/base.py
@@ -128,11 +128,7 @@ def array_for_new_object(data, specified_dtype=None):
 
 def default_lapl():
     """ Default link access property list """
-    lapl = h5p.create(h5p.LINK_ACCESS)
-    fapl = h5p.create(h5p.FILE_ACCESS)
-    fapl.set_fclose_degree(h5f.CLOSE_STRONG)
-    lapl.set_elink_fapl(fapl)
-    return lapl
+    return None
 
 
 def default_lcpl():
diff --git a/h5py/api_types_hdf5.pxd b/h5py/api_types_hdf5.pxd
index ff96b6ed..b622af45 100644
--- a/h5py/api_types_hdf5.pxd
+++ b/h5py/api_types_hdf5.pxd
@@ -115,10 +115,10 @@ cdef extern from "hdf5.h":
     H5F_SCOPE_DOWN      = 2     # for internal use only
 
   cdef enum H5F_close_degree_t:
-    H5F_CLOSE_WEAK  = 0,
-    H5F_CLOSE_SEMI  = 1,
-    H5F_CLOSE_STRONG = 2,
-    H5F_CLOSE_DEFAULT = 3
+    H5F_CLOSE_DEFAULT = 0,
+    H5F_CLOSE_WEAK    = 1,
+    H5F_CLOSE_SEMI    = 2,
+    H5F_CLOSE_STRONG  = 3
 
   cdef enum H5F_fspace_strategy_t:
     H5F_FSPACE_STRATEGY_FSM_AGGR = 0,  # FSM, Aggregators, VFD
diff --git a/h5py/h5f.pyx b/h5py/h5f.pyx
index aa50bfbb..9823fb5f 100644
--- a/h5py/h5f.pyx
+++ b/h5py/h5f.pyx
@@ -46,10 +46,10 @@ ACC_SWMR_READ  = H5F_ACC_SWMR_READ
 SCOPE_LOCAL     = H5F_SCOPE_LOCAL
 SCOPE_GLOBAL    = H5F_SCOPE_GLOBAL
 
+CLOSE_DEFAULT = H5F_CLOSE_DEFAULT
 CLOSE_WEAK  = H5F_CLOSE_WEAK
 CLOSE_SEMI  = H5F_CLOSE_SEMI
 CLOSE_STRONG = H5F_CLOSE_STRONG
-CLOSE_DEFAULT = H5F_CLOSE_DEFAULT
 
 OBJ_FILE    = H5F_OBJ_FILE
 OBJ_DATASET = H5F_OBJ_DATASET
diff --git a/h5py/h5p.pyx b/h5py/h5p.pyx
index dc8bf65a..2a163414 100644
--- a/h5py/h5p.pyx
+++ b/h5py/h5p.pyx
@@ -986,10 +986,10 @@ cdef class PropFAID(PropInstanceID):
         Set the file-close degree, which determines library behavior when
         a file is closed when objects are still open.  Legal values:
 
+        * h5f.CLOSE_DEFAULT
         * h5f.CLOSE_WEAK
         * h5f.CLOSE_SEMI
         * h5f.CLOSE_STRONG
-        * h5f.CLOSE_DEFAULT
         """
         H5Pset_fclose_degree(self.id, <H5F_close_degree_t>close_degree)
 
@@ -1001,10 +1001,10 @@ cdef class PropFAID(PropInstanceID):
         Get the file-close degree, which determines library behavior when
         a file is closed when objects are still open.  Legal values:
 
+        * h5f.CLOSE_DEFAULT
         * h5f.CLOSE_WEAK
         * h5f.CLOSE_SEMI
         * h5f.CLOSE_STRONG
-        * h5f.CLOSE_DEFAULT
         """
         cdef H5F_close_degree_t deg
         H5Pget_fclose_degree(self.id, &deg)
diff --git a/news/link_access_properties.rst b/news/link_access_properties.rst
new file mode 100644
index 00000000..c156e2f5
--- /dev/null
+++ b/news/link_access_properties.rst
@@ -0,0 +1,5 @@
+Bug fixes
+---------
+
+* Fixed values of ``H5F_close_degree_t`` enum.
+* External links are now accessed with libhdf5's default access properties.


Test patch:
diff --git a/h5py/tests/test_file.py b/h5py/tests/test_file.py
index 892b5df0..b6837228 100644
--- a/h5py/tests/test_file.py
+++ b/h5py/tests/test_file.py
@@ -1000,6 +1000,43 @@ f = h5py.File({str(filename)!r}, mode={mode!r}, locking={locking})
             assert open_in_subprocess(fname, mode="w", locking=True)
 
 
+@pytest.mark.skipif(
+    h5py.version.hdf5_version_tuple < (1, 14, 4),
+    reason="Requires HDF5 >= 1.14.4",
+)
+@pytest.mark.skipif(
+    "HDF5_USE_FILE_LOCKING" in os.environ,
+    reason="HDF5_USE_FILE_LOCKING env. var. is set",
+)
+@pytest.mark.parametrize(
+    'locking_arg,file_locking_props',
+    [
+        (False, (0, 0)),
+        (True, (1, 0)),
+        ('best-effort', (1, 1)),
+    ]
+)
+def test_file_locking_external_link(tmp_path, locking_arg, file_locking_props):
+    """Test that same file locking is used for external link"""
+    fname_main = tmp_path / "test_main.h5"
+    fname_elink = tmp_path / "test_linked.h5"
+
+    # Create test files
+    with h5py.File(fname_elink, "w") as f:
+        f["data"] = 1
+    with h5py.File(fname_main, "w") as f:
+        f["link"] = h5py.ExternalLink(fname_elink, "/data")
+
+    with h5py.File(fname_main, "r", locking=locking_arg) as f:
+        locking_info = f.id.get_access_plist().get_file_locking()
+        assert locking_info == file_locking_props
+
+        # Test that external link file is also opened without file locking
+        elink_dataset = f["link"]
+        elink_locking_info = elink_dataset.file.id.get_access_plist().get_file_locking()
+        assert elink_locking_info == file_locking_props
+
+
 def test_close_gc(writable_file):
     # https://github.com/h5py/h5py/issues/1852
     for i in range(100):