From cc50a78000a7dc536ecff0f62b7206708987bc7d Mon Sep 17 00:00:00 2001 From: jhendersonHDF Date: Fri, 1 Mar 2024 07:38:17 -0600 Subject: 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. --- test/h5test.c | 43 ++++++++++++ test/h5test.h | 1 + test/links.c | 214 ++++++++++++++++++++++++++++++++-------------------------- test/vol.c | 20 ++++++ 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; -- cgit v0.12