diff --git a/src/H5I.c b/src/H5I.c index 20eef4c708d..b97a2ef8ef1 100644 --- a/src/H5I.c +++ b/src/H5I.c @@ -2068,17 +2068,16 @@ herr_t H5I_vlock_enter(hid_t id) { /* If this attempt fails, this is undone by assignment of mod_info_k */ mod_info_k.lock_count++; - if (mod_info_k.lock_count <= 0) { + if (mod_info_k.lock_count <= 0) HGOTO_DONE(FAIL); - } /* If ID info was concurrently modified, restart and check again */ } while (!atomic_compare_exchange_strong(&(id_info_ptr->k), &info_k, mod_info_k)); - /* Validity check */ - if ((size_t) mod_info_k.lock_count > mod_info_k.app_count) { - HGOTO_DONE(FAIL); - } + + assert(mod_info_k.lock_count > 0); + /* Ensure that ID is not being used more times than app ref count allows */ + assert(mod_info_k.lock_count <= (int) mod_info_k.app_count + mod_info_k.app_unlocks ); done: FUNC_LEAVE_NOAPI(ret_value); @@ -2122,10 +2121,9 @@ herr_t H5I_vlock_exit(hid_t id) { HGOTO_DONE(SUCCEED); /* Get ID info */ - if ((id_info_ptr = H5I__find_id(id)) == NULL) { + if ((id_info_ptr = H5I__find_id(id)) == NULL) /* Assume ID was released during the course of the API routine */ HGOTO_DONE(SUCCEED); - } /* Ensure that we have exlusive write access to the ID */ do { @@ -2137,10 +2135,20 @@ herr_t H5I_vlock_exit(hid_t id) { continue; } + if (info_k.lock_count == 0) { + /* Lock count is already 0, no need to decrement */ + HGOTO_DONE(SUCCEED); + } + /* Update the lock count and check for consistency with ID ref count */ mod_info_k = info_k; - mod_info_k.lock_count--; + if (mod_info_k.app_unlocks > 0) { + /* A routine release an application-level reference, and H5I already handled the vlock release */ + mod_info_k.app_unlocks--; + } else { + mod_info_k.lock_count--; + } if (mod_info_k.lock_count < 0) HGOTO_DONE(FAIL); @@ -2148,10 +2156,9 @@ herr_t H5I_vlock_exit(hid_t id) { /* If ID info was concurrently modified, restart and check again */ } while (!atomic_compare_exchange_strong(&(id_info_ptr->k), &info_k, mod_info_k)); - /* Validity check */ - if ((size_t) mod_info_k.lock_count > mod_info_k.app_count) - HGOTO_DONE(FAIL); - + /* If app count is zero, ID was released and we don't need to worry about the lock count. + * Otherwise, ensure that the ID wasn't used more times than ref count allows */ + assert(mod_info_k.app_count == 0 ||( mod_info_k.lock_count <= ((int) mod_info_k.app_count) + mod_info_k.app_unlocks) ); done: FUNC_LEAVE_NOAPI(ret_value); } /* H5I_vlock_exit() */ diff --git a/src/H5Iint.c b/src/H5Iint.c index ade84696181..7f45a055d4b 100644 --- a/src/H5Iint.c +++ b/src/H5Iint.c @@ -2859,6 +2859,7 @@ H5I__mark_node(void *_info, void H5_ATTR_UNUSED *key, void *_udata) #if H5_HAVE_VIRTUAL_LOCK mod_info_k.lock_count = info_k.lock_count; + mod_info_k.app_unlocks = info_k.app_unlocks; #endif /* H5_HAVE_VIRTUAL_LOCK */ /* We don't want multiple threads trying to either realize or dispose of the @@ -2909,6 +2910,7 @@ H5I__mark_node(void *_info, void H5_ATTR_UNUSED *key, void *_udata) assert(info_k.have_global_mutex == mod_info_k.have_global_mutex); #if H5_HAVE_VIRTUAL_LOCK assert(info_k.lock_count == mod_info_k.lock_count); + assert(info_k.app_unlocks == mod_info_k.app_unlocks); #endif /* H5_HAVE_VIRTUAL_LOCK */ #endif /* JRM */ @@ -3094,6 +3096,7 @@ H5I__mark_node(void *_info, void H5_ATTR_UNUSED *key, void *_udata) #if H5_HAVE_VIRTUAL_LOCK mod_info_k.lock_count = 0; + mod_info_k.app_unlocks = 0; #endif /* H5_HAVE_VIRTUAL_LOCK */ } else { @@ -3108,6 +3111,7 @@ H5I__mark_node(void *_info, void H5_ATTR_UNUSED *key, void *_udata) #if H5_HAVE_VIRTUAL_LOCK mod_info_k.lock_count = info_k.lock_count; + mod_info_k.app_unlocks = info_k.app_unlocks; #endif /* H5_HAVE_VIRTUAL_LOCK */ } @@ -4067,6 +4071,7 @@ H5I_subst(hid_t id, const void *new_object) mod_info_k.object = new_object; #if H5_HAVE_VIRTUAL_LOCK mod_info_k.lock_count = info_k.lock_count; + mod_info_k.app_unlocks = info_k.app_unlocks; #endif /* H5_HAVE_VIRTUAL_LOCK */ if ( atomic_compare_exchange_strong(&(id_info_ptr->k), &info_k, mod_info_k) ) { @@ -4840,6 +4845,7 @@ H5I__remove_common(H5I_type_info_t *type_info_ptr, hid_t id) #if H5_HAVE_VIRTUAL_LOCK mod_info_k.lock_count = 0; + mod_info_k.app_unlocks = 0; #endif /* H5_HAVE_VIRTUAL_LOCK */ if ( atomic_compare_exchange_strong(&(id_info_ptr->k), &info_k, mod_info_k) ) { @@ -5270,7 +5276,15 @@ H5I__dec_ref(hid_t id, void **request, hbool_t app) mod_info_k.have_global_mutex = FALSE; #if H5_HAVE_VIRTUAL_LOCK - mod_info_k.lock_count = info_k.lock_count; + if (app && !H5I_is_default_id(id)) { + /* Application-visible reference released, mirror that in the virtual lock */ + mod_info_k.app_unlocks = info_k.app_unlocks + 1; + mod_info_k.lock_count = (info_k.lock_count > 0) ? info_k.lock_count - 1 : mod_info_k.lock_count; + } else { + /* Internal reference released, virtual lock unaffected */ + mod_info_k.lock_count = info_k.lock_count; + mod_info_k.app_unlocks = info_k.app_unlocks; + } #endif /* H5_HAVE_VIRTUAL_LOCK */ if ( info_k.count > 1 ) { @@ -5303,6 +5317,7 @@ H5I__dec_ref(hid_t id, void **request, hbool_t app) #if H5_HAVE_VIRTUAL_LOCK mod_info_k.lock_count = 0; + mod_info_k.app_unlocks = 0; #endif /* H5_HAVE_VIRTUAL_LOCK */ marked_for_deletion = TRUE; } @@ -5396,7 +5411,8 @@ H5I__dec_ref(hid_t id, void **request, hbool_t app) mod_info_k.have_global_mutex = ((have_global_mutex) || (! cls_is_mt_safe)); #if H5_HAVE_VIRTUAL_LOCK - mod_info_k.lock_count = info_k.lock_count; + mod_info_k.app_unlocks = 0; + mod_info_k.lock_count = 0; #endif /* H5_HAVE_VIRTUAL_LOCK */ /* We want to call the free function, and then mark the id for deletion. * Since we can't roll this action back, we need exclusive access to the @@ -5446,6 +5462,7 @@ H5I__dec_ref(hid_t id, void **request, hbool_t app) assert(info_k.have_global_mutex == mod_info_k.have_global_mutex); #if H5_HAVE_VIRTUAL_LOCK assert(info_k.lock_count == mod_info_k.lock_count); + assert(info_k.app_unlocks == mod_info_k.app_unlocks); #endif /* H5_HAVE_VIRTUAL_LOCK */ #endif /* JRM */ @@ -5504,6 +5521,7 @@ H5I__dec_ref(hid_t id, void **request, hbool_t app) #if H5_HAVE_VIRTUAL_LOCK mod_info_k.lock_count = 0; + mod_info_k.app_unlocks = 0; #endif /* H5_HAVE_VIRTUAL_LOCK */ marked_for_deletion = TRUE; @@ -6423,6 +6441,7 @@ H5I_inc_ref_internal(hid_t id, hbool_t app_ref) #if H5_HAVE_VIRTUAL_LOCK mod_info_k.lock_count = info_k.lock_count; + mod_info_k.app_unlocks = info_k.app_unlocks; #endif /* H5_HAVE_VIRTUAL_LOCK */ if ( app_ref ) { @@ -7100,6 +7119,7 @@ H5I__iterate_cb(void *_item, void H5_ATTR_UNUSED *_key, void *_udata) #if H5_HAVE_VIRTUAL_LOCK mod_info_k.lock_count = info_k.lock_count; + mod_info_k.app_unlocks = info_k.app_unlocks; #endif /* H5_HAVE_VIRTUAL_LOCK */ /* We want to ensure that no other thread inside H5I does anything with * the object while we call the user_func on the objec on the object. @@ -7152,6 +7172,7 @@ H5I__iterate_cb(void *_item, void H5_ATTR_UNUSED *_key, void *_udata) assert(info_k.have_global_mutex == mod_info_k.have_global_mutex); #if H5_HAVE_VIRTUAL_LOCK assert(info_k.lock_count == mod_info_k.lock_count); + assert(info_k.app_unlocks == mod_info_k.app_unlocks); #endif /* H5_HAVE_VIRTUAL_LOCK */ #endif /* JRM */ @@ -8047,6 +8068,7 @@ H5I__find_id(hid_t id) #if H5_HAVE_VIRTUAL_LOCK mod_info_k.lock_count = info_k.lock_count; + mod_info_k.app_unlocks = info_k.app_unlocks; #endif /* H5_HAVE_VIRTUAL_LOCK */ if ( ! atomic_compare_exchange_strong(&(id_info_ptr->k), &info_k, mod_info_k ) ) { @@ -8089,6 +8111,7 @@ H5I__find_id(hid_t id) #if H5_HAVE_VIRTUAL_LOCK assert(info_k.lock_count == mod_info_k.lock_count); + assert(info_k.app_unlocks == mod_info_k.app_unlocks); #endif /* H5_HAVE_VIRTUAL_LOCK */ #endif /* JRM */ @@ -9411,8 +9434,10 @@ H5I__new_mt_id_info(hid_t id, unsigned count, unsigned app_count, const void * o new_k.do_not_disturb = FALSE; new_k.is_future = is_future; new_k.have_global_mutex = FALSE; - - +#if H5_HAVE_VIRTUAL_LOCK + new_k.lock_count = 0; + new_k.app_unlocks = 0; +#endif atomic_fetch_add(&(H5I_mt_g.H5I__new_mt_id_info__num_calls), 1ULL); diff --git a/src/H5Ipkg.h b/src/H5Ipkg.h index 2f11308bc8c..c0054540d17 100644 --- a/src/H5Ipkg.h +++ b/src/H5Ipkg.h @@ -1400,6 +1400,7 @@ typedef struct H5I_mt_id_info_kernel_t { #if H5_HAVE_VIRTUAL_LOCK int lock_count; /* Virtual lock for this ID */ + int app_unlocks; /* Number of times this ID has been unlocked by the application */ #endif /* H5_HAVE_VIRTUAL_LOCK */ } H5I_mt_id_info_kernel_t; diff --git a/src/H5VL.c b/src/H5VL.c index 62b6190a764..d51dbfe05d8 100644 --- a/src/H5VL.c +++ b/src/H5VL.c @@ -492,6 +492,7 @@ H5VLunregister_connector(hid_t vol_id) hid_t native_id = H5I_INVALID_HID; herr_t ret_value = SUCCEED; /* Return value */ int dec_ref_ret = 0; /* Return value from H5I_dec_(app_)ref */ + FUNC_ENTER_API_NO_MUTEX(FAIL, vol_id) H5TRACE1("e", "i", vol_id);