Renamed Attribute in dense storage can't be deleted

[kmuehlbauer]

If I add

    /* Delete last attribute */
    ret = H5Adelete(dataset, attrname);
    CHECK(ret, FAIL, "H5Adelete");

before Dataset close in this test file, the Attribute can't be deleted. This could be another manifestation of https://forum.hdfgroup.org/t/bug-h5arename-fails-unexpectedly/4881

hdf5/test/tattr.c

Lines 2807 to 2810 in 4d45d05

	/* Close Dataset */
	ret = H5Dclose(dataset);
	CHECK(ret, FAIL, "H5Dclose");

More context:

• creation order tracking enabled
• attributes which are created in dense storage can be renamed since this fix 4e05249
• renamed attributes can't be deleted anymore
• attributes which are not renamed, can be deleted

Conclusion:

Something is going wrong in H5A__dense_rename. The above fix was introduced by @vchoi-hdfgroup, so maybe @vchoi-hdfgroup can give some insight here?

hdf5/src/H5Adense.c

Lines 858 to 1027 in 4d45d05

	*/
	herr_t
	H5A__dense_rename(H5F_t *f, const H5O_ainfo_t *ainfo, const char *old_name, const char *new_name)
	{
	H5A_bt2_ud_common_t udata; /* User data for v2 B-tree modify */
	H5HF_t * fheap = NULL; /* Fractal heap handle */
	H5HF_t * shared_fheap = NULL; /* Fractal heap handle for shared header messages */
	H5B2_t * bt2_name = NULL; /* v2 B-tree handle for name index */
	H5B2_t * bt2_corder = NULL; /* v2 B-tree handle for creation order ndex */
	H5A_t * attr_copy = NULL; /* Copy of attribute to rename */
	htri_t attr_sharable; /* Flag indicating attributes are shareable */
	htri_t shared_mesg; /* Should this message be stored in the Shared Message table? */
	hbool_t attr_exists; /* Attribute exists in v2 B-tree */
	herr_t ret_value = SUCCEED; /* Return value */
	FUNC_ENTER_PACKAGE
	/* Check arguments */
	HDassert(f);
	HDassert(ainfo);
	HDassert(old_name);
	HDassert(new_name);
	/* Check if attributes are shared in this file */
	if ((attr_sharable = H5SM_type_shared(f, H5O_ATTR_ID)) < 0)
	HGOTO_ERROR(H5E_ATTR, H5E_CANTGET, FAIL, "can't determine if attributes are shared")
	/* Get handle for shared message heap, if attributes are shareable */
	if (attr_sharable) {
	haddr_t shared_fheap_addr; /* Address of fractal heap to use */
	/* Retrieve the address of the shared message's fractal heap */
	if (H5SM_get_fheap_addr(f, H5O_ATTR_ID, &shared_fheap_addr) < 0)
	HGOTO_ERROR(H5E_ATTR, H5E_CANTGET, FAIL, "can't get shared message heap address")
	/* Check if there are any shared messages currently */
	if (H5F_addr_defined(shared_fheap_addr)) {
	/* Open the fractal heap for shared header messages */
	if (NULL == (shared_fheap = H5HF_open(f, shared_fheap_addr)))
	HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "unable to open fractal heap")
	} /* end if */
	} /* end if */
	/* Open the fractal heap */
	if (NULL == (fheap = H5HF_open(f, ainfo->fheap_addr)))
	HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "unable to open fractal heap")
	/* Open the name index v2 B-tree */
	if (NULL == (bt2_name = H5B2_open(f, ainfo->name_bt2_addr, NULL)))
	HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "unable to open v2 B-tree for name index")
	/* Create the "udata" information for v2 B-tree record modify */
	udata.f = f;
	udata.fheap = fheap;
	udata.shared_fheap = shared_fheap;
	udata.name = old_name;
	udata.name_hash = H5_checksum_lookup3(old_name, HDstrlen(old_name), 0);
	udata.flags = 0;
	udata.corder = 0;
	udata.found_op = H5A__dense_fnd_cb; /* v2 B-tree comparison callback */
	udata.found_op_data = &attr_copy;
	/* Get copy of attribute through 'name' tracking v2 B-tree */
	attr_exists = FALSE;
	if (H5B2_find(bt2_name, &udata, &attr_exists, NULL, NULL) < 0)
	HGOTO_ERROR(H5E_ATTR, H5E_NOTFOUND, FAIL, "can't search for attribute in name index")
	if (attr_exists == FALSE)
	HGOTO_ERROR(H5E_ATTR, H5E_NOTFOUND, FAIL, "can't locate attribute in name index")
	HDassert(attr_copy);
	/* Check if message is already shared */
	if ((shared_mesg = H5O_msg_is_shared(H5O_ATTR_ID, attr_copy)) < 0)
	HGOTO_ERROR(H5E_ATTR, H5E_CANTGET, FAIL, "error determining if message is shared")
	else if (shared_mesg > 0) {
	/* Reset shared status of copy */
	/* (so it will get shared again if necessary) */
	attr_copy->sh_loc.type = H5O_SHARE_TYPE_UNSHARED;
	} /* end if */
	/* Change name of attribute */
	H5MM_xfree(attr_copy->shared->name);
	attr_copy->shared->name = H5MM_xstrdup(new_name);
	/* Recompute the version to encode the attribute with */
	if (H5A__set_version(f, attr_copy) < 0)
	HGOTO_ERROR(H5E_ATTR, H5E_CANTSET, FAIL, "unable to update attribute version")
	/* Need to remove the attribute from the creation order index v2 B-tree */
	if (ainfo->index_corder) {
	hbool_t corder_attr_exists; /* Attribute exists in v2 B-tree */
	/* Open the creation order index v2 B-tree */
	HDassert(H5F_addr_defined(ainfo->corder_bt2_addr));
	if (NULL == (bt2_corder = H5B2_open(f, ainfo->corder_bt2_addr, NULL)))
	HGOTO_ERROR(H5E_ATTR, H5E_CANTOPENOBJ, FAIL, "unable to open v2 B-tree for creation index")
	/* Set up the creation order to search for */
	udata.corder = attr_copy->shared->crt_idx;
	corder_attr_exists = FALSE;
	if (H5B2_find(bt2_corder, &udata, &corder_attr_exists, NULL, NULL) < 0)
	HGOTO_ERROR(H5E_ATTR, H5E_NOTFOUND, FAIL, "can't search for attribute in name index")
	if (corder_attr_exists) {
	H5A_bt2_ud_rm_t rm_udata;
	/* Set up the creation order in user data for the v2 B-tree 'record remove' callback */
	rm_udata.common.corder = attr_copy->shared->crt_idx;
	/* Remove the record from the creation order index v2 B-tree */
	if (H5B2_remove(bt2_corder, &rm_udata, NULL, NULL) < 0)
	HGOTO_ERROR(H5E_ATTR, H5E_CANTREMOVE, FAIL,
	"unable to remove attribute from creation order index v2 B-tree")
	}
	}
	/* Insert renamed attribute back into dense storage */
	/* (Possibly making it shared) */
	if (H5A__dense_insert(f, ainfo, attr_copy) < 0)
	HGOTO_ERROR(H5E_ATTR, H5E_CANTINSERT, FAIL, "unable to add to dense storage")
	/* Was this attribute shared? */
	if ((shared_mesg = H5O_msg_is_shared(H5O_ATTR_ID, attr_copy)) > 0) {
	hsize_t attr_rc; /* Attribute's ref count in shared message storage */
	/* Retrieve ref count for shared attribute */
	if (H5SM_get_refcount(f, H5O_ATTR_ID, &attr_copy->sh_loc, &attr_rc) < 0)
	HGOTO_ERROR(H5E_ATTR, H5E_CANTGET, FAIL, "can't retrieve shared message ref count")
	/* If the newly shared attribute needs to share "ownership" of the shared
	* components (ie. its reference count is 1), increment the reference
	* count on any shared components of the attribute, so that they won't
	* be removed from the file. (Essentially a "copy on write" operation).
	*
	* *ick* -QAK, 2007/01/08
	*/
	if (attr_rc == 1) {
	/* Increment reference count on attribute components */
	if (H5O__attr_link(f, NULL, attr_copy) < 0)
	HGOTO_ERROR(H5E_ATTR, H5E_LINKCOUNT, FAIL, "unable to adjust attribute link count")
	} /* end if */
	} /* end if */
	else if (shared_mesg == 0) {
	/* Increment reference count on attribute components */
	/* (so that they aren't deleted when the attribute is removed shortly) */
	if (H5O__attr_link(f, NULL, attr_copy) < 0)
	HGOTO_ERROR(H5E_ATTR, H5E_LINKCOUNT, FAIL, "unable to adjust attribute link count")
	} /* end if */
	else if (shared_mesg < 0)
	HGOTO_ERROR(H5E_ATTR, H5E_WRITEERROR, FAIL, "error determining if message should be shared")
	/* Delete old attribute from dense storage */
	if (H5A__dense_remove(f, ainfo, old_name) < 0)
	HGOTO_ERROR(H5E_ATTR, H5E_CANTDELETE, FAIL, "unable to delete attribute in dense storage")
	done:
	/* Release resources */
	if (shared_fheap && H5HF_close(shared_fheap) < 0)
	HDONE_ERROR(H5E_ATTR, H5E_CLOSEERROR, FAIL, "can't close fractal heap")
	if (fheap && H5HF_close(fheap) < 0)
	HDONE_ERROR(H5E_ATTR, H5E_CLOSEERROR, FAIL, "can't close fractal heap")
	if (bt2_name && H5B2_close(bt2_name) < 0)
	HDONE_ERROR(H5E_ATTR, H5E_CLOSEERROR, FAIL, "can't close v2 B-tree for name index")
	if (bt2_corder && H5B2_close(bt2_corder) < 0)
	HDONE_ERROR(H5E_ATTR, H5E_CLOSEERROR, FAIL, "can't close v2 B-tree for creation order index")
	if (attr_copy)
	H5O_msg_free(H5O_ATTR_ID, attr_copy);
	FUNC_LEAVE_NOAPI(ret_value)
	} /* end H5A__dense_rename() */

[kmuehlbauer]

I've added a possible fix to #1389. The testsuite runs smoothly, and downstream works nicely too.

[epourmal]

Thank you for the proposed fix. We will be reviewing it shortly.

[kmuehlbauer]

@epourmal Thanks for the heads-up.

[kmuehlbauer]

#1389 doesn't solve the problem and I'm not able to fix this. I'd very much appreciate if HDF5 group could prioritize here to find some working solution for this annoying issue. At least a time line would be good.

HDFFV-10579

• https://forum.hdfgroup.org/t/bug-h5arename-fails-unexpectedly/4881/5

Downstream we constantly get bug reports which can be tracked to this very bug:

h5netcdf

• Modifying attributes safely is not possible with all datasets. h5netcdf/h5netcdf#207
• Segmentation fault after upgrading to h5netcdf==1.1.0 h5netcdf/h5netcdf#202
• Tests test_more_than_7_attr_creation_track_order and test_bool_slicing_length_one_dim fail in the test suite h5netcdf/h5netcdf#179
• Regression: KeyError in h5py with h5netcdf 0.13.0 h5netcdf/h5netcdf#136
• Files written by h5netcdf cannot be edited by netcdf4-python h5netcdf/h5netcdf#128

h5py

• Track order and attribute updating bug h5py/h5py#1641
• [BUG] track_order = True causes RuntimeError: Can't rename attribute (record is already in B-tree) h5py/h5py#1385
• RuntimeError: Can't rename attribute (record is already in B-tree) when building against HDF5 1.10.5 h5py/h5py#1185
• RuntimeError: Can't rename attribute (record is already in B-tree) h5py/h5py#1180

other

• Fails for groups with order tracking jjhelmus/pyfive#46

Downstream already tried to workaround that issue, but finally the problem is only shifted to a later point in hdf5-file lifetime.

C-Examples showing the problem can be found here and there.

Beside that issue, thanks for providing such a great library! 💯 👍

[kmuehlbauer]

Great! Thank you very much all involved at HDFGroup for fixing this issue. 🎉 🚀