Fix issue with FAPL file locking setting inheriting test (#4053)

Fixes an issue where the HDF5_USE_FILE_LOCKING environment variable being
set can interfere with the file locking setting that the test expects to
be returned.

4 files changed, 184 insertions, 94 deletions

diff --git a/test/h5test.c b/test/h5test.c
index 77a65c2..1cca57a 100644
--- a/test/h5test.c
+++ b/test/h5test.c
@@ -2068,6 +2068,49 @@ error:
 } /* end h5_check_if_file_locking_enabled() */
 
 /*-------------------------------------------------------------------------
+ * Function:    h5_check_file_locking_env_var
+ *
+ * Purpose:     Checks if the HDF5_USE_FILE_LOCKING file locking
+ *              environment variable is set and parses its value if so.
+ *
+ *              If the environment variable is not set, both `use_locks`
+ *              and `ignore_disabled_locks` will be set to FAIL to indicate
+ *              this. Otherwise, they will each be set appropriately based
+ *              on the setting for the environment variable.
+ *
+ * Return:      Nothing
+ *
+ *-------------------------------------------------------------------------
+ */
+void
+h5_check_file_locking_env_var(htri_t *use_locks, htri_t *ignore_disabled_locks)
+{
+    char *lock_env_var = NULL;
+
+    assert(use_locks);
+    assert(ignore_disabled_locks);
+
+    lock_env_var = getenv(HDF5_USE_FILE_LOCKING);
+    if (lock_env_var && (!strcmp(lock_env_var, "FALSE") || !strcmp(lock_env_var, "0"))) {
+        *use_locks             = false; /* Override: Never use locks */
+        *ignore_disabled_locks = FAIL;
+    }
+    else if (lock_env_var && !strcmp(lock_env_var, "BEST_EFFORT")) {
+        *use_locks             = true; /* Override: Always use locks */
+        *ignore_disabled_locks = true; /* Override: Ignore disabled locks */
+    }
+    else if (lock_env_var && (!strcmp(lock_env_var, "TRUE") || !strcmp(lock_env_var, "1"))) {
+        *use_locks             = true;  /* Override: Always use locks */
+        *ignore_disabled_locks = false; /* Override: Don't ignore disabled locks */
+    }
+    else {
+        /* Environment variable not set, or not set correctly */
+        *use_locks             = FAIL;
+        *ignore_disabled_locks = FAIL;
+    }
+}
+
+/*-------------------------------------------------------------------------
  * Function:    h5_using_native_vol
  *
  * Purpose:     Checks if the VOL connector being used is (or the VOL
diff --git a/test/h5test.h b/test/h5test.h
index 8115207..8f78567 100644
--- a/test/h5test.h
+++ b/test/h5test.h
@@ -290,6 +290,7 @@ H5TEST_DLL const char    *h5_get_version_string(H5F_libver_t libver);
 H5TEST_DLL int            h5_compare_file_bytes(char *fname1, char *fname2);
 H5TEST_DLL int            h5_duplicate_file_by_bytes(const char *orig, const char *dest);
 H5TEST_DLL herr_t         h5_check_if_file_locking_enabled(bool *are_enabled);
+H5TEST_DLL void           h5_check_file_locking_env_var(htri_t *use_locks, htri_t *ignore_disabled_locks);
 H5TEST_DLL herr_t         h5_using_native_vol(hid_t fapl_id, hid_t obj_id, bool *is_native_vol);
 H5TEST_DLL bool           h5_using_default_driver(const char *drv_name);
 H5TEST_DLL herr_t         h5_using_parallel_driver(hid_t fapl_id, bool *driver_is_parallel);
diff --git a/test/links.c b/test/links.c
index 827f1da..299a7c2 100644
--- a/test/links.c
+++ b/test/links.c
@@ -9837,24 +9837,26 @@ error:
 static int
 external_link_inherit_locking(hid_t fapl_id, bool new_format)
 {
-    hid_t fid                     = H5I_INVALID_HID;
-    hid_t tmp_fid                 = H5I_INVALID_HID;
-    hid_t gid                     = H5I_INVALID_HID;
-    hid_t ext_fid                 = H5I_INVALID_HID;
-    hid_t file_fapl               = H5I_INVALID_HID;
-    hid_t tmp_fapl                = H5I_INVALID_HID;
-    bool  use_locking             = true;
-    bool  ignore_disabled_locking = false;
-    char *filename                = NULL;
-    char *ext_filename            = NULL;
+    htri_t use_locking_env         = FAIL;
+    htri_t ignore_disabled_env     = FAIL;
+    hid_t  fid                     = H5I_INVALID_HID;
+    hid_t  tmp_fid                 = H5I_INVALID_HID;
+    hid_t  gid                     = H5I_INVALID_HID;
+    hid_t  ext_fid                 = H5I_INVALID_HID;
+    hid_t  file_fapl               = H5I_INVALID_HID;
+    hid_t  tmp_fapl                = H5I_INVALID_HID;
+    bool   use_locking             = true;
+    bool   ignore_disabled_locking = false;
+    char  *filename                = NULL;
+    char  *ext_filename            = NULL;
 
     if (new_format)
         TESTING("inheriting of file locking settings (w/new group format)");
     else
         TESTING("inheriting of file locking settings");
 
-    if (HDsetenv(HDF5_USE_FILE_LOCKING, "", 1) < 0)
-        TEST_ERROR;
+    /* Get the settings for the file locking environment variables */
+    h5_check_file_locking_env_var(&use_locking_env, &ignore_disabled_env);
 
     /* Check that external links are registered with the library */
     if (H5Lis_registered(H5L_TYPE_EXTERNAL) != true)
@@ -9884,101 +9886,125 @@ external_link_inherit_locking(hid_t fapl_id, bool new_format)
     if (H5Fclose(fid) < 0)
         TEST_ERROR;
 
-    /* Set file locking on */
-    if (H5Pset_file_locking(file_fapl, true, true) < 0)
-        TEST_ERROR;
-
-    /* Open main file */
-    if ((fid = H5Fopen(filename, H5F_ACC_RDWR, file_fapl)) < 0)
-        TEST_ERROR;
+    /* Test for file locking on unless disabled by environment variable */
+    if (use_locking_env != false) {
+        /* Set file locking on */
+        if (H5Pset_file_locking(file_fapl, true, true) < 0)
+            TEST_ERROR;
 
-    /* Make sure that locking setting retrieved from access plist
-     * matches what we set.
-     */
-    if ((tmp_fapl = H5Fget_access_plist(fid)) < 0)
-        TEST_ERROR;
-    if (H5Pget_file_locking(tmp_fapl, &use_locking, &ignore_disabled_locking) < 0)
-        TEST_ERROR;
-    if (use_locking != true || ignore_disabled_locking != true)
-        TEST_ERROR;
-    if (H5Pclose(tmp_fapl) < 0)
-        TEST_ERROR;
+        /* Open main file */
+        if ((fid = H5Fopen(filename, H5F_ACC_RDWR, file_fapl)) < 0)
+            TEST_ERROR;
 
-    /* Open external file through link */
-    if ((gid = H5Gopen2(fid, "ext_link", H5P_DEFAULT)) < 0)
-        TEST_ERROR;
+        /* Make sure that locking setting retrieved from access plist
+         * matches what we set.
+         */
+        if ((tmp_fapl = H5Fget_access_plist(fid)) < 0)
+            TEST_ERROR;
+        if (H5Pget_file_locking(tmp_fapl, &use_locking, &ignore_disabled_locking) < 0)
+            TEST_ERROR;
+        if (use_locking != true)
+            TEST_ERROR;
+        /* Check for "ignore disabled file locks" setting being on, unless
+         * disabled by environment variable
+         */
+        if (ignore_disabled_env != false && ignore_disabled_locking != true)
+            TEST_ERROR;
+        if (H5Pclose(tmp_fapl) < 0)
+            TEST_ERROR;
 
-    /* Get file ID for external file */
-    if ((tmp_fid = H5Iget_file_id(gid)) < 0)
-        TEST_ERROR;
+        /* Open external file through link */
+        if ((gid = H5Gopen2(fid, "ext_link", H5P_DEFAULT)) < 0)
+            TEST_ERROR;
 
-    /* Make sure that locking setting retrieved from external file's
-     * access plist matches what we set.
-     */
-    if ((tmp_fapl = H5Fget_access_plist(tmp_fid)) < 0)
-        TEST_ERROR;
-    if (H5Pget_file_locking(tmp_fapl, &use_locking, &ignore_disabled_locking) < 0)
-        TEST_ERROR;
-    if (use_locking != true || ignore_disabled_locking != true)
-        TEST_ERROR;
-    if (H5Pclose(tmp_fapl) < 0)
-        TEST_ERROR;
+        /* Get file ID for external file */
+        if ((tmp_fid = H5Iget_file_id(gid)) < 0)
+            TEST_ERROR;
 
-    if (H5Gclose(gid) < 0)
-        TEST_ERROR;
-    if (H5Fclose(tmp_fid) < 0)
-        TEST_ERROR;
-    if (H5Fclose(fid) < 0)
-        TEST_ERROR;
+        /* Make sure that locking setting retrieved from external file's
+         * access plist matches what we set.
+         */
+        if ((tmp_fapl = H5Fget_access_plist(tmp_fid)) < 0)
+            TEST_ERROR;
+        if (H5Pget_file_locking(tmp_fapl, &use_locking, &ignore_disabled_locking) < 0)
+            TEST_ERROR;
+        if (use_locking != true)
+            TEST_ERROR;
+        /* Check for "ignore disabled file locks" setting being on, unless
+         * disabled by environment variable
+         */
+        if (ignore_disabled_env != false && ignore_disabled_locking != true)
+            TEST_ERROR;
+        if (H5Pclose(tmp_fapl) < 0)
+            TEST_ERROR;
 
-    /* Repeat with file locking off */
+        if (H5Gclose(gid) < 0)
+            TEST_ERROR;
+        if (H5Fclose(tmp_fid) < 0)
+            TEST_ERROR;
+        if (H5Fclose(fid) < 0)
+            TEST_ERROR;
+    }
 
-    /* Set file locking off */
-    if (H5Pset_file_locking(file_fapl, false, false) < 0)
-        TEST_ERROR;
+    /* Test for file locking off unless force enabled by environment variable */
+    if (use_locking_env != true) {
+        /* Set file locking off */
+        if (H5Pset_file_locking(file_fapl, false, false) < 0)
+            TEST_ERROR;
 
-    /* Open main file */
-    if ((fid = H5Fopen(filename, H5F_ACC_RDWR, file_fapl)) < 0)
-        TEST_ERROR;
+        /* Open main file */
+        if ((fid = H5Fopen(filename, H5F_ACC_RDWR, file_fapl)) < 0)
+            TEST_ERROR;
 
-    /* Make sure that locking setting retrieved from access plist
-     * matches what we set.
-     */
-    if ((tmp_fapl = H5Fget_access_plist(fid)) < 0)
-        TEST_ERROR;
-    if (H5Pget_file_locking(tmp_fapl, &use_locking, &ignore_disabled_locking) < 0)
-        TEST_ERROR;
-    if (use_locking != false || ignore_disabled_locking != false)
-        TEST_ERROR;
-    if (H5Pclose(tmp_fapl) < 0)
-        TEST_ERROR;
+        /* Make sure that locking setting retrieved from access plist
+         * matches what we set.
+         */
+        if ((tmp_fapl = H5Fget_access_plist(fid)) < 0)
+            TEST_ERROR;
+        if (H5Pget_file_locking(tmp_fapl, &use_locking, &ignore_disabled_locking) < 0)
+            TEST_ERROR;
+        if (use_locking != false)
+            TEST_ERROR;
+        /* Check for "ignore disabled file locks" setting being off, unless
+         * force enabled by environment variable
+         */
+        if (ignore_disabled_env != true && ignore_disabled_locking != false)
+            TEST_ERROR;
+        if (H5Pclose(tmp_fapl) < 0)
+            TEST_ERROR;
 
-    /* Open external file through link */
-    if ((gid = H5Gopen2(fid, "ext_link", H5P_DEFAULT)) < 0)
-        TEST_ERROR;
+        /* Open external file through link */
+        if ((gid = H5Gopen2(fid, "ext_link", H5P_DEFAULT)) < 0)
+            TEST_ERROR;
 
-    /* Get file ID for external file */
-    if ((tmp_fid = H5Iget_file_id(gid)) < 0)
-        TEST_ERROR;
+        /* Get file ID for external file */
+        if ((tmp_fid = H5Iget_file_id(gid)) < 0)
+            TEST_ERROR;
 
-    /* Make sure that locking setting retrieved from external file's
-     * access plist matches what we set.
-     */
-    if ((tmp_fapl = H5Fget_access_plist(tmp_fid)) < 0)
-        TEST_ERROR;
-    if (H5Pget_file_locking(tmp_fapl, &use_locking, &ignore_disabled_locking) < 0)
-        TEST_ERROR;
-    if (use_locking != false || ignore_disabled_locking != false)
-        TEST_ERROR;
-    if (H5Pclose(tmp_fapl) < 0)
-        TEST_ERROR;
+        /* Make sure that locking setting retrieved from external file's
+         * access plist matches what we set.
+         */
+        if ((tmp_fapl = H5Fget_access_plist(tmp_fid)) < 0)
+            TEST_ERROR;
+        if (H5Pget_file_locking(tmp_fapl, &use_locking, &ignore_disabled_locking) < 0)
+            TEST_ERROR;
+        if (use_locking != false)
+            TEST_ERROR;
+        /* Check for "ignore disabled file locks" setting being off, unless
+         * force enabled by environment variable
+         */
+        if (ignore_disabled_env != true && ignore_disabled_locking != false)
+            TEST_ERROR;
+        if (H5Pclose(tmp_fapl) < 0)
+            TEST_ERROR;
 
-    if (H5Gclose(gid) < 0)
-        TEST_ERROR;
-    if (H5Fclose(tmp_fid) < 0)
-        TEST_ERROR;
-    if (H5Fclose(fid) < 0)
-        TEST_ERROR;
+        if (H5Gclose(gid) < 0)
+            TEST_ERROR;
+        if (H5Fclose(tmp_fid) < 0)
+            TEST_ERROR;
+        if (H5Fclose(fid) < 0)
+            TEST_ERROR;
+    }
 
     if (H5Fdelete(ext_filename, file_fapl) < 0)
         TEST_ERROR;
diff --git a/test/vol.c b/test/vol.c
index 43336c6..e29c6bb 100644
--- a/test/vol.c
+++ b/test/vol.c
@@ -865,6 +865,8 @@ test_basic_file_operation(const char *env_h5_drvr)
     hid_t fapl_id2   = H5I_INVALID_HID;
     hid_t fcpl_id    = H5I_INVALID_HID;
 
+    htri_t      use_locking_env     = FAIL;
+    htri_t      ignore_disabled_env = FAIL;
     char        filename[1024];
     ssize_t     obj_count;
     hid_t       obj_id_list[1];
@@ -894,6 +896,24 @@ test_basic_file_operation(const char *env_h5_drvr)
     if (H5Pset_metadata_read_attempts(fapl_id, 9) < 0)
         TEST_ERROR;
 
+    /* Similar to the above, make sure the FAPL has an appropriate file locking
+     * setting if the HDF5_USE_FILE_LOCKING environment variable was set so that
+     * the H5Pequal call will work correctly.
+     */
+    h5_check_file_locking_env_var(&use_locking_env, &ignore_disabled_env);
+    if (use_locking_env != FAIL) {
+        hbool_t default_use_locking           = true;
+        hbool_t default_ignore_disabled_locks = true;
+
+        if (H5Pget_file_locking(H5P_DEFAULT, &default_use_locking, &default_ignore_disabled_locks) < 0)
+            TEST_ERROR;
+
+        if (H5Pset_file_locking(fapl_id, (bool)use_locking_env,
+                                (ignore_disabled_env == FAIL) ? default_ignore_disabled_locks
+                                                              : (bool)ignore_disabled_env) < 0)
+            TEST_ERROR;
+    }
+
     /* H5Fcreate */
     if ((fid = H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, fapl_id)) < 0)
         TEST_ERROR;