HDFGroup / hdf5

Official HDF5® Library Repository
https://www.hdfgroup.org/
Other
609 stars 247 forks source link

Crash when copying a group with references to another file #3429

Open roosre opened 1 year ago

roosre commented 1 year ago

Describe the bug HDF5 crashes when copy "/composite_cae/Meshes/M1/Element_Types" of the linked file at the end of this report. The crash occurs with hdf5 1.12 and 1.14 when using the c++ API, h5py or the hdf5 tools.

"C:\Program Files\HDF_Group\HDF5\1.14.1\bin\h5copy.exe" -v -f ref -i "D:\tmp\input_mech.h5" -s "/composite_cae/" -o "D:\tmp\output.h5" -d "/copy"

target_h5 = h5py.File(r'D:\tmp\target.h5','w') target_h5.copy(source=composit_cae_group, dest=target_h5, name="/copy", shallow=False, expand_soft=False, expand_external=False, expand_refs=True, without_attrs=False)



The crash occurs also when using c++ H5OCopy.

**Expected behavior**
The group is copied to the file and the references are updated correctly

**Platform (please complete the following information)**
 - HDF5 version: 1.12 and 1.14
 - OS and version: Windows 11
 - Compiler and version: VS C++17
 - Build system (e.g. CMake, Autotools) and version: premake5
 - Any configure options you specified:
 - MPI library and version (parallel HDF5): serial

**Additional context**
The crash disappears if the mesh (number of elements and nodes) is smaller.

**Attachements**
[Linked file](https://drive.google.com/file/d/1PLub8PTYb6Bki-1NrCPhCXwa8cO8MrDY/view?usp=sharing)
jhendersonHDF commented 1 year ago

Hi @roosre,

at first glance, it looks like you may be running into an issue similar to the one in https://github.com/HDFGroup/hdf5/issues/3335. That issue deals with variable-length types, but I believe reference types may have a similar problem. Would it be possible to try copying your file using the h5repack HDF5 tool rather than the h5copy tool? There is some logic in h5repack to avoid using H5Ocopy and instead manually copy over objects with a reference or variable-length type, so I believe you may be able to avoid the crash with h5repack. You likely won't be able to quite get equivalent functionality between h5copy and h5repack, but if h5repack works it would at least let us know that the same logic for avoiding H5Ocopy probably needs to be moved back to the h5copy tool as well.

That said, I believe we need to add a note/warning to the H5Ocopy documentation that it currently has issues with objects that have certain datatypes.

roosre commented 1 year ago

Hi @jhendersonHDF , thanks for the answer. h5repack works but unfortunately we have to copy the group to a certain path in an existing file. So we cannot use repack. The provided example was just the most simple case I could find how to reproduce the issue. Thanks.

jhendersonHDF commented 1 year ago

Thanks for the confirmation that h5repack works @roosre! This means that h5copy will need to be updated with similar logic to what's found in h5repack to manually copy objects over when they have a variable-length or reference type instead of using H5Ocopy. Unfortunately there isn't a good timeline on this issue currently, but hopefully we'll get this fixed for the next release of HDF5 1.14.

roosre commented 1 year ago

Hi @jhendersonHDF , I had a look at #3331 were h5repack is fixed. We are using H5Ocopy in the c++ backend and I am wondering what the suggested solution is for this workflow? Thanks, René

jhendersonHDF commented 1 year ago

Until we can determine what changes need to be made to H5Ocopy to make it always work correctly for objects with a variable-length or reference datatype, I'd say it would probably be best to manually re-create those objects in the same way that h5repack does. Essentially that would entail following https://github.com/HDFGroup/hdf5/blob/develop/tools/src/h5repack/h5repack_copy.c#L802-L844 to check if the dataset has a problematic type and then using whatever is needed out of https://github.com/HDFGroup/hdf5/blob/develop/tools/src/h5repack/h5repack_copy.c#L882-L1276 to re-create the dataset with H5Dcreate2 and then copy over data with H5Dread/H5Dwrite.

Of course H5Ocopy is much more convenient, so I hope that we can fix that soon, but at first glance it seemed that the changes might be a bit involved.