mirrors/hdf5
2
0
Fork 0
mirror of https://github.com/HDFGroup/hdf5.git synced 2026-09-25 04:09:44 +03:00
Code Issues Packages Projects Releases Wiki Activity

Restore FUNC_ENTER_API_NOINIT_NOLOCK for H5Zconfig_get_* functions

Browse Source 
Revert fad067a1c1, which replaced these with the regular locking
FUNC_ENTER_API_NOINIT on the premise that "the API lock is already
recursive." That holds for HDF5_ENABLE_THREADSAFE (a genuinely
recursive mutex) but not for HDF5_ENABLE_CONCURRENCY, whose H5_API_LOCK
uses a per-call dlftt (disable-locking-for-this-thread) counter that
each FUNC_ENTER_API_NOINIT call site initializes fresh rather than a
mutex that recognizes same-thread reentry. Since H5Zconfig_get_int and
friends are called from inside H5Z_set_config_func_t callbacks that
run under H5Pappend_filter's already-held API lock, the second lock
acquisition self-deadlocks under concurrency builds.

Reproduced directly: built locally with -DHDF5_ENABLE_CONCURRENCY=ON
(matching CI's Concurrency Workflows config) and confirmed tfilter2
hangs indefinitely on the first filter that invokes set_config with a
real config string; after this revert it completes in the same run.
This is what was actually causing the H5TEST-tfilter2 and
H5REPACK_UD-plugin_test_cfg timeouts under CI's Concurrency Workflows.
This commit is contained in:
Scot Breitenfeld
2026-07-16 00:07:29 -05:00
parent 936a847d06
commit 39d0bb9102
2 changed files with 48 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/H5Zconfig.c
+17 -17
View File
@@ -492,16 +492,16 @@ H5Zconfig_has_key(const char *params, const char *key)
toml_datum_t d;
htri_t ret_value = FAIL;
/* The API lock is recursive, so this is safe to call from inside an
* H5Z_set_config_func_t callback that is already running under the API
* lock held by H5Pappend_filter. */
FUNC_ENTER_API_NOINIT
/* No API lock: this is a pure parser over caller-provided buffers and
* may be called from inside an H5Z_set_config_func_t callback that is
* already running under the API lock held by H5Pappend_filter. */
FUNC_ENTER_API_NOINIT_NOLOCK
ret_value = H5Z__config_get_datum(params, key, &tr, &d);
if (ret_value > 0)
toml_free(tr);
FUNC_LEAVE_API_NOINIT(ret_value)
FUNC_LEAVE_API_NOINIT_NOLOCK(ret_value)
}
/*-------------------------------------------------------------------------
@@ -578,12 +578,12 @@ H5Zconfig_get_int(const char *params, const char *key, int64_t *out)
{
htri_t ret_value = FAIL;
/* See comment on H5Zconfig_has_key about recursive re-entry. */
FUNC_ENTER_API_NOINIT
/* No API lock: see comment on H5Zconfig_has_key. */
FUNC_ENTER_API_NOINIT_NOLOCK
ret_value = H5Z__config_get_int(params, key, out);
FUNC_LEAVE_API_NOINIT(ret_value)
FUNC_LEAVE_API_NOINIT_NOLOCK(ret_value)
}
/*-------------------------------------------------------------------------
@@ -606,8 +606,8 @@ H5Zconfig_get_double(const char *params, const char *key, double *out)
htri_t found;
htri_t ret_value = FAIL;
/* See comment on H5Zconfig_has_key about recursive re-entry. */
FUNC_ENTER_API_NOINIT
/* No API lock: see comment on H5Zconfig_has_key. */
FUNC_ENTER_API_NOINIT_NOLOCK
if (!out)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "out must not be NULL");
@@ -628,7 +628,7 @@ H5Zconfig_get_double(const char *params, const char *key, double *out)
done:
if (tr_valid)
toml_free(tr);
FUNC_LEAVE_API_NOINIT(ret_value)
FUNC_LEAVE_API_NOINIT_NOLOCK(ret_value)
}
/*-------------------------------------------------------------------------
@@ -650,8 +650,8 @@ H5Zconfig_get_bool(const char *params, const char *key, bool *out)
htri_t found;
htri_t ret_value = FAIL;
/* See comment on H5Zconfig_has_key about recursive re-entry. */
FUNC_ENTER_API_NOINIT
/* No API lock: see comment on H5Zconfig_has_key. */
FUNC_ENTER_API_NOINIT_NOLOCK
if (!out)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "out must not be NULL");
@@ -669,7 +669,7 @@ H5Zconfig_get_bool(const char *params, const char *key, bool *out)
done:
if (tr_valid)
toml_free(tr);
FUNC_LEAVE_API_NOINIT(ret_value)
FUNC_LEAVE_API_NOINIT_NOLOCK(ret_value)
}
/*-------------------------------------------------------------------------
@@ -763,10 +763,10 @@ H5Zconfig_get_str(const char *params, const char *key, char *buf, size_t *buf_si
{
htri_t ret_value = FAIL;
/* See comment on H5Zconfig_has_key about recursive re-entry. */
FUNC_ENTER_API_NOINIT
/* No API lock: see comment on H5Zconfig_has_key. */
FUNC_ENTER_API_NOINIT_NOLOCK
ret_value = H5Z__config_get_str(params, key, buf, buf_size);
FUNC_LEAVE_API_NOINIT(ret_value)
FUNC_LEAVE_API_NOINIT_NOLOCK(ret_value)
}
src/H5private.h
+31
View File
@@ -1453,6 +1453,26 @@ extern char H5_lib_vers_info_g[];
H5_API_LOCK \
{
/*
* Use this macro for public API functions that may be re-entered from inside
* a library callback that already holds the API lock (e.g. filter v3
* H5Z_set_config_func_t callbacks invoked from H5Pappend_filter). Performs
* the same error-handling and function-name setup as FUNC_ENTER_API_NOINIT
* but skips H5_API_LOCK, so the function is safe to call while another
* public API frame already holds the lock. Only appropriate for pure
* helpers that touch no global library state. Examples: H5Zconfig_get_int,
* H5Zconfig_get_double, etc.
*/
#define FUNC_ENTER_API_NOINIT_NOLOCK \
{ \
{ \
{ \
H5_CHECK_FUNCTION_NAME(H5_IS_PUBLIC(__func__)); \
\
H5_API_SETUP_PUBLIC_API_VARS \
H5_API_SETUP_ERROR_HANDLING \
{
/*
* Use this macro for public API functions that shouldn't perform _any_
* initialization of the library or an interface or push themselves on the
@@ -1676,6 +1696,17 @@ extern char H5_lib_vers_info_g[];
} \
} /* end scope from beginning of FUNC_ENTER */
/* Use this macro to match the FUNC_ENTER_API_NOINIT_NOLOCK macro */
#define FUNC_LEAVE_API_NOINIT_NOLOCK(ret_value) \
; \
} /* end scope from end of FUNC_ENTER */ \
if (H5_UNLIKELY(err_occurred)) \
(void)H5E_dump_api_stack(); \
return (ret_value); \
} \
} \
} /* end scope from beginning of FUNC_ENTER */
/* Use this macro to match the FUNC_ENTER_API_NOINIT_NOERR macro */
#define FUNC_LEAVE_API_NOERR(ret_value) \
; \