Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove cached datatype conversion path table entries on file close #3942

Merged
merged 2 commits into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/H5Fint.c
Original file line number Diff line number Diff line change
Expand Up @@ -1615,6 +1615,18 @@ H5F__dest(H5F_t *f, bool flush, bool free_on_failure)
if (vol_wrap_ctx && (NULL == H5VL_object_unwrap(f->vol_obj)))
HDONE_ERROR(H5E_FILE, H5E_CANTGET, FAIL, "can't unwrap VOL object");

/*
* Clean up any cached type conversion path table entries that
* may have been keeping a reference to the file's VOL object
* in order to prevent the file from being closed out from
* underneath other places that may access the conversion path
* or its src/dst datatypes later on (currently, conversions on
* variable-length and reference datatypes involve this)
*/
if (H5T_unregister(H5T_PERS_SOFT, NULL, NULL, NULL, f->vol_obj, NULL) < 0)
HDONE_ERROR(H5E_FILE, H5E_CANTRELEASE, FAIL,
"unable to free cached type conversion path table entries");

if (H5VL_free_object(f->vol_obj) < 0)
HDONE_ERROR(H5E_FILE, H5E_CANTDEC, FAIL, "unable to free VOL object");
f->vol_obj = NULL;
Expand Down
112 changes: 100 additions & 12 deletions src/H5T.c
Original file line number Diff line number Diff line change
Expand Up @@ -343,12 +343,13 @@ typedef H5T_t *(*H5T_copy_func_t)(H5T_t *old_dt);
static herr_t H5T__register_int(H5T_pers_t pers, const char *name, H5T_t *src, H5T_t *dst,
H5T_lib_conv_t func);
static herr_t H5T__register(H5T_pers_t pers, const char *name, H5T_t *src, H5T_t *dst, H5T_conv_func_t *conv);
static herr_t H5T__unregister(H5T_pers_t pers, const char *name, H5T_t *src, H5T_t *dst, H5T_conv_t func);
static htri_t H5T__compiler_conv(H5T_t *src, H5T_t *dst);
static herr_t H5T__set_size(H5T_t *dt, size_t size);
static herr_t H5T__close_cb(H5T_t *dt, void **request);
static H5T_path_t *H5T__path_find_real(const H5T_t *src, const H5T_t *dst, const char *name,
H5T_conv_func_t *conv);
static bool H5T_path_match(H5T_path_t *path, H5T_pers_t pers, const char *name, H5T_t *src, H5T_t *dst,
H5VL_object_t *owned_vol_obj, H5T_conv_t func);
static bool H5T__detect_vlen_ref(const H5T_t *dt);
static H5T_t *H5T__initiate_copy(const H5T_t *old_dt);
static H5T_t *H5T__copy_transient(H5T_t *old_dt);
Expand Down Expand Up @@ -2672,7 +2673,7 @@ H5Tregister(H5T_pers_t pers, const char *name, hid_t src_id, hid_t dst_id, H5T_c
} /* end H5Tregister() */

/*-------------------------------------------------------------------------
* Function: H5T__unregister
* Function: H5T_unregister
*
* Purpose: Removes conversion paths that match the specified criteria.
* All arguments are optional. Missing arguments are wild cards.
Expand All @@ -2683,18 +2684,33 @@ H5Tregister(H5T_pers_t pers, const char *name, hid_t src_id, hid_t dst_id, H5T_c
*
*-------------------------------------------------------------------------
*/
static herr_t
H5T__unregister(H5T_pers_t pers, const char *name, H5T_t *src, H5T_t *dst, H5T_conv_t func)
herr_t
H5T_unregister(H5T_pers_t pers, const char *name, H5T_t *src, H5T_t *dst, H5VL_object_t *owned_vol_obj,
H5T_conv_t func)
{
H5T_path_t *path = NULL; /*conversion path */
H5T_soft_t *soft = NULL; /*soft conversion information */
int nprint = 0; /*number of paths shut down */
int i; /*counter */

FUNC_ENTER_PACKAGE_NOERR
FUNC_ENTER_NOAPI_NOERR

/* Remove matching entries from the soft list */
if (H5T_PERS_DONTCARE == pers || H5T_PERS_SOFT == pers) {
/*
* Remove matching entries from the soft list if:
*
* - The caller didn't specify a particular type (soft or hard)
* of conversion path to match against or specified that soft
* conversion paths should be matched against
*
* AND
*
* - The caller didn't provide the `owned_vol_obj` parameter;
* if this parameter is provided, we want to leave the soft
* list untouched and only remove cached conversion paths
* below where the file VOL object associated with the path's
* source or destination types matches the given VOL object.
*/
if ((H5T_PERS_DONTCARE == pers || H5T_PERS_SOFT == pers) && !owned_vol_obj) {
for (i = H5T_g.nsoft - 1; i >= 0; --i) {
soft = H5T_g.soft + i;
assert(soft);
Expand All @@ -2714,13 +2730,15 @@ H5T__unregister(H5T_pers_t pers, const char *name, H5T_t *src, H5T_t *dst, H5T_c

/* Remove matching conversion paths, except no-op path */
for (i = H5T_g.npaths - 1; i > 0; --i) {
bool nomatch;

path = H5T_g.path[i];
assert(path);

nomatch = !H5T_path_match(path, pers, name, src, dst, owned_vol_obj, func);

/* Not a match */
if (((H5T_PERS_SOFT == pers && path->is_hard) || (H5T_PERS_HARD == pers && !path->is_hard)) ||
(name && *name && strcmp(name, path->name) != 0) || (src && H5T_cmp(src, path->src, false)) ||
(dst && H5T_cmp(dst, path->dst, false)) || (func && func != path->conv.u.app_func)) {
if (nomatch) {
/*
* Notify all other functions to recalculate private data since some
* functions might cache a list of conversion functions. For
Expand Down Expand Up @@ -2769,7 +2787,7 @@ H5T__unregister(H5T_pers_t pers, const char *name, H5T_t *src, H5T_t *dst, H5T_c
} /* end for */

FUNC_LEAVE_NOAPI(SUCCEED)
} /* end H5T__unregister() */
} /* end H5T_unregister() */

/*-------------------------------------------------------------------------
* Function: H5Tunregister
Expand Down Expand Up @@ -2799,7 +2817,7 @@ H5Tunregister(H5T_pers_t pers, const char *name, hid_t src_id, hid_t dst_id, H5T
if (dst_id > 0 && (NULL == (dst = (H5T_t *)H5I_object_verify(dst_id, H5I_DATATYPE))))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "dst is not a data type");

if (H5T__unregister(pers, name, src, dst, func) < 0)
if (H5T_unregister(pers, name, src, dst, NULL, func) < 0)
HGOTO_ERROR(H5E_DATATYPE, H5E_CANTDELETE, FAIL, "internal unregister function failed");

done:
Expand Down Expand Up @@ -5149,6 +5167,53 @@ H5T__path_find_real(const H5T_t *src, const H5T_t *dst, const char *name, H5T_co
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5T__path_find_real() */

/*-------------------------------------------------------------------------
* Function: H5T_path_match
*
* Purpose: Helper function to determine whether a datatype conversion
* path object matches against a given set of criteria.
*
* Return: true/false (can't fail)
*
*-------------------------------------------------------------------------
*/
static bool
H5T_path_match(H5T_path_t *path, H5T_pers_t pers, const char *name, H5T_t *src, H5T_t *dst,
H5VL_object_t *owned_vol_obj, H5T_conv_t func)
{
bool ret_value = true;

assert(path);

FUNC_ENTER_NOAPI_NOINIT_NOERR

if (
/* Check that the specified conversion function persistence matches */
((H5T_PERS_SOFT == pers && path->is_hard) || (H5T_PERS_HARD == pers && !path->is_hard)) ||

/* Check that the specified conversion path name matches */
(name && *name && strcmp(name, path->name) != 0) ||

/*
* Check that the specified source and destination datatypes match
* the source and destination datatypes in the conversion path
*/
(src && H5T_cmp(src, path->src, false)) || (dst && H5T_cmp(dst, path->dst, false)) ||

/*
* Check that the specified VOL object matches the VOL object
* in the conversion path
*/
(owned_vol_obj && (owned_vol_obj != path->src->shared->owned_vol_obj) &&
(owned_vol_obj != path->dst->shared->owned_vol_obj)) ||

/* Check that the specified conversion function matches */
(func && func != path->conv.u.app_func))
ret_value = false;

FUNC_LEAVE_NOAPI(ret_value)
} /* H5T_path_match() */

/*-------------------------------------------------------------------------
* Function: H5T_path_noop
*
Expand Down Expand Up @@ -6096,3 +6161,26 @@ H5T_own_vol_obj(H5T_t *dt, H5VL_object_t *vol_obj)
done:
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5T_own_vol_obj() */

/*-------------------------------------------------------------------------
* Function: H5T__get_path_table_npaths
*
* Purpose: Testing function to return the number of type conversion
* paths currently stored in the type conversion path table
* cache.
*
* Return: Number of type conversion paths (can't fail)
*
*-------------------------------------------------------------------------
*/
int
H5T__get_path_table_npaths(void)
{
int ret_value = 0;

FUNC_ENTER_PACKAGE_NOERR

ret_value = H5T_g.npaths;

FUNC_LEAVE_NOAPI(ret_value)
}
3 changes: 3 additions & 0 deletions src/H5Tpkg.h
Original file line number Diff line number Diff line change
Expand Up @@ -877,4 +877,7 @@ H5_DLL herr_t H5T__sort_name(const H5T_t *dt, int *map);
/* Debugging functions */
H5_DLL herr_t H5T__print_stats(H5T_path_t *path, int *nprint /*in,out*/);

/* Testing functions */
H5_DLL int H5T__get_path_table_npaths(void);

#endif /* H5Tpkg_H */
2 changes: 2 additions & 0 deletions src/H5Tprivate.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@ H5_DLL H5T_path_t *H5T_path_find(const H5T_t *src, const H5T_t *dst);
H5_DLL bool H5T_path_noop(const H5T_path_t *p);
H5_DLL H5T_bkg_t H5T_path_bkg(const H5T_path_t *p);
H5_DLL H5T_subset_info_t *H5T_path_compound_subset(const H5T_path_t *p);
H5_DLL herr_t H5T_unregister(H5T_pers_t pers, const char *name, H5T_t *src, H5T_t *dst,
H5VL_object_t *owned_vol_obj, H5T_conv_t func);
H5_DLL herr_t H5T_convert(H5T_path_t *tpath, hid_t src_id, hid_t dst_id, size_t nelmts, size_t buf_stride,
size_t bkg_stride, void *buf, void *bkg);
H5_DLL herr_t H5T_reclaim(hid_t type_id, struct H5S_t *space, void *buf);
Expand Down
Loading