diff options
author | Binh-Minh Ribler <bmribler@hdfgroup.org> | 2015-04-06 03:52:35 (GMT) |
---|---|---|
committer | Binh-Minh Ribler <bmribler@hdfgroup.org> | 2015-04-06 03:52:35 (GMT) |
commit | 2a2a79742dc2ee154b7571a518b97413599b29a6 (patch) | |
tree | cda2e53402608cdf22eb2be41d27c6bb63c32ad9 | |
parent | 150b85cc44903c005009552d692da50a4aa6c86f (diff) | |
download | hdf5-2a2a79742dc2ee154b7571a518b97413599b29a6.zip hdf5-2a2a79742dc2ee154b7571a518b97413599b29a6.tar.gz hdf5-2a2a79742dc2ee154b7571a518b97413599b29a6.tar.bz2 |
[svn-r26731] Purpose: Fixed HDFFV-7947
Description:
When copy constructor or constructor that takes an existing id is invoked,
the C ref counter stays the same but there is an extra C++ object which
later is destroyed and may cause the HDF5 id to be closed prematurely. The
C++ library needs to increment the ref counter in these situations, so that
the C library will not close the id when it is still being referenced.
However, the incrementing of ref count left some objects opened at the end
of the program, perhaps, due to compiler's optimization on cons/destructors. The constructor, that takes an existing id, needs to increment the counter
but it seems that the matching destructor wasn't invoked. The workaround
is to have a function for each class that has "id" that only sets the id
and not increment the ref count for the library to use in these situations.
These functions are "friend" and not public.
The friend functions are:
void f_Attribute_setId(Attribute *, hid_t)
void f_DataSet_setId(DataSet *, hid_t)
void f_DataSpace_setId(DataSpace *, hid_t)
void f_DataType_setId(DataType *, hid_t)
Merged from trunk: r26655
Platforms tested:
Linux/64 (platypus)
Linux/32 2.6 (jam Intel 15.0)
SunOS 5.11 (emu)
-rw-r--r-- | c++/src/H5AbstractDs.cpp | 41 | ||||
-rw-r--r-- | c++/src/H5AbstractDs.h | 1 | ||||
-rw-r--r-- | c++/src/H5ArrayType.h | 1 | ||||
-rw-r--r-- | c++/src/H5Attribute.cpp | 5 | ||||
-rw-r--r-- | c++/src/H5Attribute.h | 23 | ||||
-rw-r--r-- | c++/src/H5CommonFG.cpp | 75 | ||||
-rw-r--r-- | c++/src/H5CommonFG.h | 4 | ||||
-rw-r--r-- | c++/src/H5CompType.cpp | 10 | ||||
-rw-r--r-- | c++/src/H5DataSet.cpp | 7 | ||||
-rw-r--r-- | c++/src/H5DataSet.h | 15 | ||||
-rw-r--r-- | c++/src/H5DataSpace.h | 3 | ||||
-rw-r--r-- | c++/src/H5DataType.cpp | 6 | ||||
-rw-r--r-- | c++/src/H5DataType.h | 3 | ||||
-rw-r--r-- | c++/src/H5IdComponent.cpp | 5 | ||||
-rw-r--r-- | c++/src/H5IdComponent.h | 1 | ||||
-rw-r--r-- | c++/src/H5Location.cpp | 58 | ||||
-rw-r--r-- | c++/src/H5Location.h | 4 | ||||
-rw-r--r-- | c++/src/H5PropList.cpp | 1 | ||||
-rw-r--r-- | c++/src/H5VarLenType.h | 1 | ||||
-rw-r--r-- | c++/test/tattr.cpp | 16 | ||||
-rw-r--r-- | c++/test/ttypes.cpp | 21 |
21 files changed, 225 insertions, 76 deletions
diff --git a/c++/src/H5AbstractDs.cpp b/c++/src/H5AbstractDs.cpp index 0e6ac00..d59c1eb 100644 --- a/c++/src/H5AbstractDs.cpp +++ b/c++/src/H5AbstractDs.cpp @@ -21,6 +21,7 @@ #include "H5PropList.h" #include "H5Object.h" #include "H5AbstractDs.h" +#include "H5DataSpace.h" #include "H5DcreatProp.h" #include "H5CommonFG.h" #include "H5Alltypes.h" @@ -124,8 +125,9 @@ DataType AbstractDs::getDataType() const // depending on which object invokes getDataType. Then, create and // return the DataType object try { - DataType datatype(p_get_type()); - return(datatype); + DataType datatype; + f_DataType_setId(&datatype, p_get_type()); + return(datatype); } catch (DataSetIException E) { throw DataTypeIException("DataSet::getDataType", E.getDetailMsg()); @@ -150,8 +152,9 @@ ArrayType AbstractDs::getArrayType() const // depending on which object invokes getArrayType. Then, create and // return the ArrayType object try { - ArrayType arraytype(p_get_type()); - return(arraytype); + ArrayType arraytype; + f_DataType_setId(&arraytype, p_get_type()); + return(arraytype); } catch (DataSetIException E) { throw DataTypeIException("DataSet::getArrayType", E.getDetailMsg()); @@ -176,8 +179,9 @@ CompType AbstractDs::getCompType() const // depending on which object invokes getCompType. Then, create and // return the CompType object try { - CompType comptype(p_get_type()); - return(comptype); + CompType comptype; + f_DataType_setId(&comptype, p_get_type()); + return(comptype); } catch (DataSetIException E) { throw DataTypeIException("DataSet::getCompType", E.getDetailMsg()); @@ -202,8 +206,9 @@ EnumType AbstractDs::getEnumType() const // depending on which object invokes getEnumType. Then, create and // return the EnumType object try { - EnumType enumtype(p_get_type()); - return(enumtype); + EnumType enumtype; + f_DataType_setId(&enumtype, p_get_type()); + return(enumtype); } catch (DataSetIException E) { throw DataTypeIException("DataSet::getEnumType", E.getDetailMsg()); @@ -228,8 +233,9 @@ IntType AbstractDs::getIntType() const // depending on which object invokes getIntType. Then, create and // return the IntType object try { - IntType inttype(p_get_type()); - return(inttype); + IntType inttype; + f_DataType_setId(&inttype, p_get_type()); + return(inttype); } catch (DataSetIException E) { throw DataTypeIException("DataSet::getIntType", E.getDetailMsg()); @@ -254,8 +260,9 @@ FloatType AbstractDs::getFloatType() const // depending on which object invokes getFloatType. Then, create and // return the FloatType object try { - FloatType floatype(p_get_type()); - return(floatype); + FloatType floatype; + f_DataType_setId(&floatype, p_get_type()); + return(floatype); } catch (DataSetIException E) { throw DataTypeIException("DataSet::getFloatType", E.getDetailMsg()); @@ -280,8 +287,9 @@ StrType AbstractDs::getStrType() const // depending on which object invokes getStrType. Then, create and // return the StrType object try { - StrType strtype(p_get_type()); - return(strtype); + StrType strtype; + f_DataType_setId(&strtype, p_get_type()); + return(strtype); } catch (DataSetIException E) { throw DataTypeIException("DataSet::getStrType", E.getDetailMsg()); @@ -306,8 +314,9 @@ VarLenType AbstractDs::getVarLenType() const // depending on which object invokes getVarLenType. Then, create and // return the VarLenType object try { - VarLenType varlentype(p_get_type()); - return(varlentype); + VarLenType varlentype; + f_DataType_setId(&varlentype, p_get_type()); + return(varlentype); } catch (DataSetIException E) { throw DataTypeIException("DataSet::getVarLenType", E.getDetailMsg()); diff --git a/c++/src/H5AbstractDs.h b/c++/src/H5AbstractDs.h index 8e5ac0e..ee2e45e 100644 --- a/c++/src/H5AbstractDs.h +++ b/c++/src/H5AbstractDs.h @@ -28,6 +28,7 @@ class FloatType; class IntType; class StrType; class VarLenType; +class DataSpace; /*! \class AbstractDs \brief AbstractDs is an abstract base class, inherited by Attribute diff --git a/c++/src/H5ArrayType.h b/c++/src/H5ArrayType.h index 511126e..6577a6e 100644 --- a/c++/src/H5ArrayType.h +++ b/c++/src/H5ArrayType.h @@ -49,7 +49,6 @@ class H5_DLLCPP ArrayType : public DataType { // Noop destructor virtual ~ArrayType(); - protected: // Default constructor ArrayType(); diff --git a/c++/src/H5Attribute.cpp b/c++/src/H5Attribute.cpp index 8b43e80..12f39df 100644 --- a/c++/src/H5Attribute.cpp +++ b/c++/src/H5Attribute.cpp @@ -270,8 +270,9 @@ DataSpace Attribute::getSpace() const // If the dataspace id is valid, create and return the DataSpace object if( dataspace_id > 0 ) { - DataSpace dataspace( dataspace_id ); - return( dataspace ); + DataSpace dataspace; + f_DataSpace_setId(&dataspace, dataspace_id); + return(dataspace); } else { diff --git a/c++/src/H5Attribute.h b/c++/src/H5Attribute.h index 33c4db3..f936f04 100644 --- a/c++/src/H5Attribute.h +++ b/c++/src/H5Attribute.h @@ -31,6 +31,16 @@ namespace H5 { */ class H5_DLLCPP Attribute : public AbstractDs, public IdComponent { public: + + // Copy constructor: makes a copy of an existing Attribute object. + Attribute( const Attribute& original ); + + // Default constructor + Attribute(); + + // Creates a copy of an existing attribute using the attribute id + Attribute( const hid_t attr_id ); + // Closes this attribute. virtual void close(); @@ -70,15 +80,6 @@ class H5_DLLCPP Attribute : public AbstractDs, public IdComponent { ///\brief Returns this class name. virtual H5std_string fromClass () const { return("Attribute"); } - // Creates a copy of an existing attribute using the attribute id - Attribute( const hid_t attr_id ); - - // Copy constructor: makes a copy of an existing Attribute object. - Attribute( const Attribute& original ); - - // Default constructor - Attribute(); - // Gets the attribute id. virtual hid_t getId() const; @@ -107,6 +108,10 @@ class H5_DLLCPP Attribute : public AbstractDs, public IdComponent { // do not inherit H5Object::renameAttr void renameAttr() {} + + // Friend function to set Attribute id. For library use only. + friend void f_Attribute_setId(Attribute* attr, hid_t new_id); + }; #ifndef H5_NO_NAMESPACE } diff --git a/c++/src/H5CommonFG.cpp b/c++/src/H5CommonFG.cpp index 1ef36eb..1547a5b 100644 --- a/c++/src/H5CommonFG.cpp +++ b/c++/src/H5CommonFG.cpp @@ -14,6 +14,7 @@ * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ #include <string> +#include <iostream> #include "H5Include.h" #include "H5Exception.h" @@ -33,9 +34,6 @@ #include "H5Alltypes.h" #include "H5private.h" // for HDstrcpy -#include <iostream> -using namespace std; - // There are a few comments that are common to most of the functions // defined in this file so they are listed here. // - getLocId is called by all functions, that call a C API, to get @@ -51,6 +49,7 @@ using namespace std; #ifndef H5_NO_NAMESPACE namespace H5 { +using namespace std; #endif //-------------------------------------------------------------------------- @@ -100,7 +99,9 @@ Group CommonFG::createGroup( const char* name, size_t size_hint ) const throwException("createGroup", "H5Gcreate2 failed"); // No failure, create and return the Group object - Group group( group_id ); + Group group; + CommonFG *ptr = &group; + ptr->p_setId(group_id); return( group ); } @@ -136,7 +137,9 @@ Group CommonFG::openGroup( const char* name ) const throwException("openGroup", "H5Gopen2 failed"); // No failure, create and return the Group object - Group group( group_id ); + Group group; + CommonFG *ptr = &group; + ptr->p_setId(group_id); return( group ); } @@ -178,7 +181,8 @@ DataSet CommonFG::createDataSet( const char* name, const DataType& data_type, co throwException("createDataSet", "H5Dcreate2 failed"); // No failure, create and return the DataSet object - DataSet dataset( dataset_id ); + DataSet dataset; + f_DataSet_setId(&dataset, dataset_id); return( dataset ); } @@ -213,7 +217,8 @@ DataSet CommonFG::openDataSet( const char* name ) const throwException("openDataSet", "H5Dopen2 failed"); // No failure, create and return the DataSet object - DataSet dataset( dataset_id ); + DataSet dataset; + f_DataSet_setId(&dataset, dataset_id); return( dataset ); } @@ -576,7 +581,8 @@ DataType CommonFG::openDataType( const char* name ) const throwException("openDataType", "H5Topen2 failed"); // No failure, create and return the DataType object - DataType data_type(type_id); + DataType data_type; + f_DataType_setId(&data_type, type_id); return(data_type); } @@ -611,7 +617,8 @@ ArrayType CommonFG::openArrayType( const char* name ) const throwException("openArrayType", "H5Topen2 failed"); // No failure, create and return the ArrayType object - ArrayType array_type (type_id); + ArrayType array_type; + f_DataType_setId(&array_type, type_id); return(array_type); } @@ -646,7 +653,8 @@ CompType CommonFG::openCompType( const char* name ) const throwException("openCompType", "H5Topen2 failed"); // No failure, create and return the CompType object - CompType comp_type(type_id); + CompType comp_type; + f_DataType_setId(&comp_type, type_id); return(comp_type); } @@ -681,7 +689,8 @@ EnumType CommonFG::openEnumType( const char* name ) const throwException("openEnumType", "H5Topen2 failed"); // No failure, create and return the EnumType object - EnumType enum_type(type_id); + EnumType enum_type; + f_DataType_setId(&enum_type, type_id); return(enum_type); } @@ -716,7 +725,8 @@ IntType CommonFG::openIntType( const char* name ) const throwException("openIntType", "H5Topen2 failed"); // No failure, create and return the IntType object - IntType int_type(type_id); + IntType int_type; + f_DataType_setId(&int_type, type_id); return(int_type); } @@ -751,7 +761,8 @@ FloatType CommonFG::openFloatType( const char* name ) const throwException("openFloatType", "H5Topen2 failed"); // No failure, create and return the FloatType object - FloatType float_type(type_id); + FloatType float_type; + f_DataType_setId(&float_type, type_id); return(float_type); } @@ -786,7 +797,8 @@ StrType CommonFG::openStrType( const char* name ) const throwException("openStrType", "H5Topen2 failed"); // No failure, create and return the StrType object - StrType str_type(type_id); + StrType str_type; + f_DataType_setId(&str_type, type_id); return(str_type); } @@ -821,7 +833,8 @@ VarLenType CommonFG::openVarLenType( const char* name ) const throwException("openVarLenType", "H5Topen2 failed"); // No failure, create and return the VarLenType object - VarLenType varlen_type(type_id); + VarLenType varlen_type; + f_DataType_setId(&varlen_type, type_id); return(varlen_type); } @@ -1224,6 +1237,7 @@ H5G_obj_t CommonFG::getObjTypeByIdx(hsize_t idx, H5std_string& type_name) const } return (obj_type); } + #endif // DOXYGEN_SHOULD_SKIP_THIS #endif /* H5_NO_DEPRECATED_SYMBOLS */ @@ -1241,6 +1255,37 @@ CommonFG::CommonFG() {} // Programmer Binh-Minh Ribler - 2000 //-------------------------------------------------------------------------- CommonFG::~CommonFG() {} + +//-------------------------------------------------------------------------- +// Function: f_DataType_setId - friend +// Purpose: This function is friend to class H5::DataType so that it +// can set DataType::id in order to work around a problem +// described in the JIRA issue HDFFV-7947. +// Applications shouldn't need to use it. +// param dtype - IN/OUT: DataType object to be changed +// param new_id - IN: New id to set +// Programmer Binh-Minh Ribler - 2015 +//-------------------------------------------------------------------------- +void f_DataType_setId(DataType* dtype, hid_t new_id) +{ + dtype->id = new_id; +} + +//-------------------------------------------------------------------------- +// Function: f_DataSet_setId - friend +// Purpose: This function is friend to class H5::DataSet so that it +// can set DataSet::id in order to work around a problem +// described in the JIRA issue HDFFV-7947. +// Applications shouldn't need to use it. +// param dset - IN/OUT: DataSet object to be changed +// param new_id - IN: New id to set +// Programmer Binh-Minh Ribler - 2015 +//-------------------------------------------------------------------------- +void f_DataSet_setId(DataSet* dset, hid_t new_id) +{ + dset->id = new_id; +} + #endif // DOXYGEN_SHOULD_SKIP_THIS #ifndef H5_NO_NAMESPACE diff --git a/c++/src/H5CommonFG.h b/c++/src/H5CommonFG.h index ab1a849..d36d78c 100644 --- a/c++/src/H5CommonFG.h +++ b/c++/src/H5CommonFG.h @@ -165,6 +165,10 @@ class H5_DLLCPP CommonFG { // Noop destructor. virtual ~CommonFG(); + + protected: + virtual void p_setId(const hid_t new_id) = 0; + #endif // DOXYGEN_SHOULD_SKIP_THIS }; // end of CommonFG declaration diff --git a/c++/src/H5CompType.cpp b/c++/src/H5CompType.cpp index 191f004..393aafc 100644 --- a/c++/src/H5CompType.cpp +++ b/c++/src/H5CompType.cpp @@ -227,7 +227,8 @@ hid_t CompType::p_get_member_type(unsigned member_num) const DataType CompType::getMemberDataType( unsigned member_num ) const { try { - DataType datatype(p_get_member_type(member_num)); + DataType datatype; + f_DataType_setId(&datatype, p_get_member_type(member_num)); return(datatype); } catch (DataTypeIException E) { @@ -248,6 +249,7 @@ ArrayType CompType::getMemberArrayType( unsigned member_num ) const { try { ArrayType arraytype(p_get_member_type(member_num)); + f_DataType_setId(&arraytype, p_get_member_type(member_num)); return(arraytype); } catch (DataTypeIException E) { @@ -268,6 +270,7 @@ CompType CompType::getMemberCompType( unsigned member_num ) const { try { CompType comptype(p_get_member_type(member_num)); + f_DataType_setId(&comptype, p_get_member_type(member_num)); return(comptype); } catch (DataTypeIException E) { @@ -288,6 +291,7 @@ EnumType CompType::getMemberEnumType( unsigned member_num ) const { try { EnumType enumtype(p_get_member_type(member_num)); + f_DataType_setId(&enumtype, p_get_member_type(member_num)); return(enumtype); } catch (DataTypeIException E) { @@ -308,6 +312,7 @@ IntType CompType::getMemberIntType( unsigned member_num ) const { try { IntType inttype(p_get_member_type(member_num)); + f_DataType_setId(&inttype, p_get_member_type(member_num)); return(inttype); } catch (DataTypeIException E) { @@ -328,6 +333,7 @@ FloatType CompType::getMemberFloatType( unsigned member_num ) const { try { FloatType floatype(p_get_member_type(member_num)); + f_DataType_setId(&floatype, p_get_member_type(member_num)); return(floatype); } catch (DataTypeIException E) { @@ -348,6 +354,7 @@ StrType CompType::getMemberStrType( unsigned member_num ) const { try { StrType strtype(p_get_member_type(member_num)); + f_DataType_setId(&strtype, p_get_member_type(member_num)); return(strtype); } catch (DataTypeIException E) { @@ -368,6 +375,7 @@ VarLenType CompType::getMemberVarLenType( unsigned member_num ) const { try { VarLenType varlentype(p_get_member_type(member_num)); + f_DataType_setId(&varlentype, p_get_member_type(member_num)); return(varlentype); } catch (DataTypeIException E) { diff --git a/c++/src/H5DataSet.cpp b/c++/src/H5DataSet.cpp index b0f49bf..8917f91 100644 --- a/c++/src/H5DataSet.cpp +++ b/c++/src/H5DataSet.cpp @@ -135,7 +135,8 @@ DataSpace DataSet::getSpace() const throw DataSetIException("DataSet::getSpace", "H5Dget_space failed"); } //create dataspace object using the existing id then return the object - DataSpace data_space( dataspace_id ); + DataSpace data_space; + f_DataSpace_setId(&data_space, dataspace_id); return( data_space ); } @@ -168,8 +169,8 @@ DSetCreatPropList DataSet::getCreatePlist() const throw DataSetIException("DataSet::getCreatePlist", "H5Dget_create_plist failed"); } // create and return the DSetCreatPropList object - DSetCreatPropList create_plist( create_plist_id ); - return( create_plist ); + DSetCreatPropList create_plist(create_plist_id); // ok to use existing id const + return(create_plist); } //-------------------------------------------------------------------------- diff --git a/c++/src/H5DataSet.h b/c++/src/H5DataSet.h index e264751..a539d6b 100644 --- a/c++/src/H5DataSet.h +++ b/c++/src/H5DataSet.h @@ -30,6 +30,7 @@ namespace H5 { */ class H5_DLLCPP DataSet : public H5Object, public AbstractDs { public: + // Close this dataset. virtual void close(); @@ -107,6 +108,12 @@ class H5_DLLCPP DataSet : public H5Object, public AbstractDs { // Destructor: properly terminates access to this dataset. virtual ~DataSet(); + protected: +#ifndef DOXYGEN_SHOULD_SKIP_THIS + // Sets the dataset id. + virtual void p_setId(const hid_t new_id); +#endif // DOXYGEN_SHOULD_SKIP_THIS + private: hid_t id; // HDF5 dataset id @@ -120,11 +127,9 @@ class H5_DLLCPP DataSet : public H5Object, public AbstractDs { void p_read_fixed_len(const hid_t mem_type_id, const hid_t mem_space_id, const hid_t file_space_id, const hid_t xfer_plist_id, H5std_string& strg) const; void p_read_variable_len(const hid_t mem_type_id, const hid_t mem_space_id, const hid_t file_space_id, const hid_t xfer_plist_id, H5std_string& strg) const; - protected: -#ifndef DOXYGEN_SHOULD_SKIP_THIS - // Sets the dataset id. - virtual void p_setId(const hid_t new_id); -#endif // DOXYGEN_SHOULD_SKIP_THIS + // Friend function to set DataSet id. For library use only. + friend void f_DataSet_setId(DataSet* dset, hid_t new_id); + }; #ifndef H5_NO_NAMESPACE } diff --git a/c++/src/H5DataSpace.h b/c++/src/H5DataSpace.h index fd4b1d6..48575a1 100644 --- a/c++/src/H5DataSpace.h +++ b/c++/src/H5DataSpace.h @@ -130,6 +130,9 @@ class H5_DLLCPP DataSpace : public IdComponent { virtual void p_setId(const hid_t new_id); #endif // DOXYGEN_SHOULD_SKIP_THIS + + // Friend function to set DataSpace id. For library use only. + friend void f_DataSpace_setId(DataSpace *dspace, hid_t new_id); }; #ifndef H5_NO_NAMESPACE } diff --git a/c++/src/H5DataType.cpp b/c++/src/H5DataType.cpp index ba7493c..aed0dd1 100644 --- a/c++/src/H5DataType.cpp +++ b/c++/src/H5DataType.cpp @@ -71,6 +71,7 @@ DataType::DataType() : H5Object(), id(H5I_INVALID_HID) {} DataType::DataType(const hid_t existing_id) : H5Object() { id = existing_id; + incRefCount(); // increment number of references to this id } //-------------------------------------------------------------------------- @@ -488,8 +489,9 @@ DataType DataType::getSuper() const // the base type, otherwise, raise exception if( base_type_id > 0 ) { - DataType base_type( base_type_id ); - return( base_type ); + DataType base_type; + base_type.p_setId(base_type_id); + return(base_type); } else { diff --git a/c++/src/H5DataType.h b/c++/src/H5DataType.h index a120736..69f59b1 100644 --- a/c++/src/H5DataType.h +++ b/c++/src/H5DataType.h @@ -139,6 +139,9 @@ class H5_DLLCPP DataType : public H5Object { #endif // DOXYGEN_SHOULD_SKIP_THIS private: + // Friend function to set DataType id. For library use only. + friend void f_DataType_setId(DataType* dtype, hid_t new_id); + void p_commit(hid_t loc_id, const char* name); }; #ifndef H5_NO_NAMESPACE diff --git a/c++/src/H5IdComponent.cpp b/c++/src/H5IdComponent.cpp index 54813ea..c01d41e 100644 --- a/c++/src/H5IdComponent.cpp +++ b/c++/src/H5IdComponent.cpp @@ -46,6 +46,11 @@ namespace H5 { //-------------------------------------------------------------------------- IdComponent::IdComponent(const hid_t h5_id) {} +//void IdComponent::p_setId(const hid_t new_id) +//{ + //p_setId(new_id); +//} + //-------------------------------------------------------------------------- // Function: IdComponent copy constructor // Purpose: This noop copy constructor is removed as a result of the data diff --git a/c++/src/H5IdComponent.h b/c++/src/H5IdComponent.h index 8e1e572..c959e95 100644 --- a/c++/src/H5IdComponent.h +++ b/c++/src/H5IdComponent.h @@ -107,6 +107,7 @@ class H5_DLLCPP IdComponent { // Sets the identifier of this object to a new value. - this one // doesn't increment reference count virtual void p_setId(const hid_t new_id) = 0; + //virtual void p_setId(const hid_t new_id); #endif // DOXYGEN_SHOULD_SKIP_THIS diff --git a/c++/src/H5Location.cpp b/c++/src/H5Location.cpp index a3a79e0..83a89d3 100644 --- a/c++/src/H5Location.cpp +++ b/c++/src/H5Location.cpp @@ -126,8 +126,9 @@ Attribute H5Location::createAttribute( const char* name, const DataType& data_ty // If the attribute id is valid, create and return the Attribute object if( attr_id > 0 ) { - Attribute attr( attr_id ); - return( attr ); + Attribute attr; + f_Attribute_setId(&attr, attr_id); + return( attr ); } else throw AttributeIException(inMemFunc("createAttribute"), "H5Acreate2 failed"); @@ -158,8 +159,9 @@ Attribute H5Location::openAttribute( const char* name ) const hid_t attr_id = H5Aopen(getId(), name, H5P_DEFAULT); if( attr_id > 0 ) { - Attribute attr( attr_id ); - return( attr ); + Attribute attr; + f_Attribute_setId(&attr, attr_id); + return( attr ); } else { @@ -193,12 +195,13 @@ Attribute H5Location::openAttribute( const unsigned int idx ) const H5_ITER_INC, (hsize_t)idx, H5P_DEFAULT, H5P_DEFAULT); if( attr_id > 0 ) { - Attribute attr( attr_id ); - return( attr ); + Attribute attr( attr_id ); + f_Attribute_setId(&attr, attr_id); + return(attr); } else { - throw AttributeIException(inMemFunc("openAttribute"), "H5Aopen_by_idx failed"); + throw AttributeIException(inMemFunc("openAttribute"), "H5Aopen_by_idx failed"); } } @@ -895,6 +898,12 @@ H5O_type_t H5Location::p_get_ref_obj_type(void *ref, H5R_type_t ref_type) const ///\return DataSpace object ///\exception H5::ReferenceException // Programmer Binh-Minh Ribler - May, 2004 +// Modification +// Mar 29, 2015 +// Used friend function to set id for DataSpace instead of the +// existing id constructor or the setId method to avoid incrementing +// ref count, as a work-around for a problem described in the JIRA +// issue HDFFV-7947. -BMR //-------------------------------------------------------------------------- DataSpace H5Location::getRegion(void *ref, H5R_type_t ref_type) const { @@ -904,8 +913,9 @@ DataSpace H5Location::getRegion(void *ref, H5R_type_t ref_type) const throw ReferenceException(inMemFunc("getRegion"), "H5Rget_region failed"); } try { - DataSpace dataspace(space_id); - return(dataspace); + DataSpace dataspace; + f_DataSpace_setId(&dataspace, space_id); + return(dataspace); } catch (DataSpaceIException E) { throw ReferenceException(inMemFunc("getRegion"), E.getDetailMsg()); @@ -920,6 +930,36 @@ DataSpace H5Location::getRegion(void *ref, H5R_type_t ref_type) const //-------------------------------------------------------------------------- H5Location::~H5Location() {} +//-------------------------------------------------------------------------- +// Function: f_Attribute_setId - friend +// Purpose: This function is friend to class H5::Attribute so that it +// can set Attribute::id in order to work around a problem +// described in the JIRA issue HDFFV-7947. +// Applications shouldn't need to use it. +// param attr - IN/OUT: Attribute object to be changed +// param new_id - IN: New id to set +// Programmer Binh-Minh Ribler - 2015 +//-------------------------------------------------------------------------- +void f_Attribute_setId(Attribute* attr, hid_t new_id) +{ + attr->id = new_id; +} + +//-------------------------------------------------------------------------- +// Function: f_DataSpace_setId - friend +// Purpose: This function is friend to class H5::DataSpace so that it can +// can set DataSpace::id in order to work around a problem +// described in the JIRA issue HDFFV-7947. +// Applications shouldn't need to use it. +// param dspace - IN/OUT: DataSpace object to be changed +// param new_id - IN: New id to set +// Programmer Binh-Minh Ribler - 2015 +//-------------------------------------------------------------------------- +void f_DataSpace_setId(DataSpace* dspace, hid_t new_id) +{ + dspace->id = new_id; +} + #ifndef H5_NO_NAMESPACE } // end namespace #endif diff --git a/c++/src/H5Location.h b/c++/src/H5Location.h index 9c9ecec..dc6ae51 100644 --- a/c++/src/H5Location.h +++ b/c++/src/H5Location.h @@ -165,6 +165,10 @@ class H5_DLLCPP H5Location : public IdComponent { // Retrieves the type of object that an object reference points to. H5O_type_t p_get_ref_obj_type(void *ref, H5R_type_t ref_type) const; + // Sets the identifier of this object to a new value. - this one + // doesn't increment reference count + virtual void p_setId(const hid_t new_id) = 0; + #endif // DOXYGEN_SHOULD_SKIP_THIS // Noop destructor. diff --git a/c++/src/H5PropList.cpp b/c++/src/H5PropList.cpp index 1d7d1ba..70ec629 100644 --- a/c++/src/H5PropList.cpp +++ b/c++/src/H5PropList.cpp @@ -258,6 +258,7 @@ void PropList::p_setId(const hid_t new_id) // reset object's id to the given id id = new_id; } + #endif // DOXYGEN_SHOULD_SKIP_THIS //-------------------------------------------------------------------------- diff --git a/c++/src/H5VarLenType.h b/c++/src/H5VarLenType.h index 9ad2b40..0a67d12 100644 --- a/c++/src/H5VarLenType.h +++ b/c++/src/H5VarLenType.h @@ -43,7 +43,6 @@ class H5_DLLCPP VarLenType : public DataType { // Noop destructor virtual ~VarLenType(); - protected: // Default constructor VarLenType(); }; diff --git a/c++/test/tattr.cpp b/c++/test/tattr.cpp index c604637..275a287 100644 --- a/c++/test/tattr.cpp +++ b/c++/test/tattr.cpp @@ -741,12 +741,26 @@ static void test_attr_compound_read() // Verify name H5std_string attr_name = attr.getName(); verify_val(attr_name, ATTR4_NAME, "Attribute::getName", __LINE__, __FILE__); - PASSED(); } // end try block catch (Exception E) { issue_fail_msg("test_attr_compound_read()", __LINE__, __FILE__, E.getCDetailMsg()); } + + try + { + // Now, try truncating the file to make sure reference counting is good. + // If any references to ids in the previous block are left unterminated, + // the truncating will fail, because the file will not be closed in + // the file.close() above. + H5File file1(FILE_COMPOUND, H5F_ACC_TRUNC); + + PASSED(); + } // end try block + + catch (FileIException E) { + issue_fail_msg("test_attr_compound_read()", __LINE__, __FILE__, "Unable to truncate file, possibly because some objects are left opened"); + } } // test_attr_compound_read() /**************************************************************** diff --git a/c++/test/ttypes.cpp b/c++/test/ttypes.cpp index 39f14a4..9f85613 100644 --- a/c++/test/ttypes.cpp +++ b/c++/test/ttypes.cpp @@ -280,10 +280,16 @@ static void test_query() tid2.close(); file.close(); + // Try truncating the file to make sure reference counting is good. + // If any references to ids of tid1 and tid2 are left unterminated, + // the truncating will fail, because the file will not be closed in + // the file.close() above. + H5File file1(FILENAME[2], H5F_ACC_TRUNC); + PASSED(); } // end of try block catch (Exception E) { - issue_fail_msg("test_query", __LINE__, __FILE__, E.getCDetailMsg()); + issue_fail_msg("test_query", __LINE__, __FILE__, E.getCDetailMsg()); } } // test_query @@ -464,27 +470,20 @@ static void test_named () trans_type.setPrecision(256); trans_type.close(); - /* - * Close the committed type and reopen it. It should return a named type. -* This had something to do with the way IntType was returned and assigned -and caused itype.committed not working correctly. So, use another_type for -now. + // Close the committed type and reopen it. It should be a named type. itype.close(); itype = file.openIntType("native-int"); iscommitted = itype.committed(); -*/ - IntType another_type = file.openIntType("native-int"); - iscommitted = another_type.committed(); if (!iscommitted) throw InvalidActionException("IntType::committed()", "Opened named types should be named types!"); // Create a dataset that uses the named type, then get the dataset's // datatype and make sure it's a named type. - DataSet dset = file.createDataSet("dset1", another_type, space); + DataSet dset = file.createDataSet("dset1", itype, space); ds_type = new DataType(dset.getDataType()); iscommitted = ds_type->committed(); if (!iscommitted) - throw InvalidActionException("IntType::committed()", "1 Dataset type should be named type!"); + throw InvalidActionException("IntType::committed()", "Dataset type should be named type!"); dset.close(); ds_type->close(); delete ds_type; |