From deec486732b7c8646cc0a2614a62b85fff336fe0 Mon Sep 17 00:00:00 2001 From: Binh-Minh Ribler Date: Tue, 23 May 2006 12:59:40 -0500 Subject: [svn-r12368] Purpose: Fixed bug Description: Shanti compiler destroy unnamed objects later than others, which caused some reference counting test fail. Revised the test so that destructors are called at the same time, regardless the differences of compiler implementation. Revised some constructors, close, operator=, and destructors to make sure that all the object ids are handled properly. Platforms tested: Linux 2.4 (heping) SunOS 5.9 (shanti) HPUX 11.00 (kelgia) AIX 5.1 (copper) --- c++/src/H5Attribute.cpp | 30 ++++++++++--------- c++/src/H5DataSet.cpp | 24 ++++++++------- c++/src/H5DataSpace.cpp | 57 +++++++++++++++++------------------ c++/src/H5DataType.cpp | 32 +++++++++++--------- c++/src/H5File.cpp | 30 ++++++++++--------- c++/src/H5Group.cpp | 23 ++++++++------- c++/src/H5IdComponent.cpp | 75 +++++++++++++++++++++-------------------------- c++/src/H5Object.cpp | 2 +- c++/src/H5PredType.cpp | 7 +++-- c++/src/H5PropList.cpp | 49 +++++++++++++++---------------- 10 files changed, 166 insertions(+), 163 deletions(-) diff --git a/c++/src/H5Attribute.cpp b/c++/src/H5Attribute.cpp index 666de18..7547ca5 100644 --- a/c++/src/H5Attribute.cpp +++ b/c++/src/H5Attribute.cpp @@ -270,13 +270,16 @@ H5std_string Attribute::getName() const //-------------------------------------------------------------------------- void Attribute::close() { - herr_t ret_value = H5Aclose(id); - if( ret_value < 0 ) - { - throw AttributeIException("Attribute::close", "H5Aclose failed"); - } - // reset the id because the attribute that it represents is now closed - id = 0; + if (p_valid_id(id)) + { + herr_t ret_value = H5Aclose(id); + if( ret_value < 0 ) + { + throw AttributeIException("Attribute::close", "H5Aclose failed"); + } + // reset the id because the attribute that it represents is now closed + id = 0; + } } //-------------------------------------------------------------------------- @@ -304,13 +307,12 @@ hsize_t Attribute::getStorageSize() const //-------------------------------------------------------------------------- Attribute::~Attribute() { - // The attribute id will be closed properly - try { - decRefCount(); - } - catch (Exception close_error) { - cerr << "Attribute::~Attribute - " << close_error.getDetailMsg() << endl; - } + try { + close(); + } + catch (Exception close_error) { + cerr << "Attribute::~Attribute - " << close_error.getDetailMsg() << endl; + } } #ifndef H5_NO_NAMESPACE diff --git a/c++/src/H5DataSet.cpp b/c++/src/H5DataSet.cpp index 12da925..fe03008 100644 --- a/c++/src/H5DataSet.cpp +++ b/c++/src/H5DataSet.cpp @@ -46,7 +46,7 @@ namespace H5 { ///\brief Default constructor: creates a stub DataSet. // Programmer Binh-Minh Ribler - 2000 //-------------------------------------------------------------------------- -DataSet::DataSet() : AbstractDs() {} +DataSet::DataSet() : AbstractDs() {} //-------------------------------------------------------------------------- // Function: DataSet overloaded constructor @@ -521,13 +521,16 @@ DataSpace DataSet::getRegion(void *ref, H5R_type_t ref_type) const //-------------------------------------------------------------------------- void DataSet::close() { - herr_t ret_value = H5Dclose( id ); - if( ret_value < 0 ) - { - throw DataSetIException("DataSet::close", "H5Dclose failed"); - } - // reset the id because the dataset that it represents is now closed - id = 0; + if (p_valid_id(id)) + { + herr_t ret_value = H5Dclose( id ); + if( ret_value < 0 ) + { + throw DataSetIException("DataSet::close", "H5Dclose failed"); + } + // reset the id because the dataset that it represents is now closed + id = 0; + } } //-------------------------------------------------------------------------- @@ -540,12 +543,11 @@ void DataSet::close() //-------------------------------------------------------------------------- DataSet::~DataSet() { - // The dataset id will be closed properly try { - decRefCount(); + close(); } catch (Exception close_error) { - cerr << "DataSet::~DataSet - " << close_error.getDetailMsg() << endl; + cerr << "DataSet::~DataSet - " << close_error.getDetailMsg() << endl; } } diff --git a/c++/src/H5DataSpace.cpp b/c++/src/H5DataSpace.cpp index c7a098f..e786144 100644 --- a/c++/src/H5DataSpace.cpp +++ b/c++/src/H5DataSpace.cpp @@ -46,7 +46,7 @@ const DataSpace DataSpace::ALL( H5S_ALL ); ///\exception H5::DataSpaceIException // Programmer Binh-Minh Ribler - 2000 //-------------------------------------------------------------------------- -DataSpace::DataSpace( H5S_class_t type ) : IdComponent() +DataSpace::DataSpace( H5S_class_t type ) : IdComponent(0) { id = H5Screate( type ); if( id < 0 ) @@ -64,7 +64,7 @@ DataSpace::DataSpace( H5S_class_t type ) : IdComponent() ///\exception H5::DataSpaceIException // Programmer Binh-Minh Ribler - 2000 //-------------------------------------------------------------------------- -DataSpace::DataSpace( int rank, const hsize_t * dims, const hsize_t * maxdims) : IdComponent() +DataSpace::DataSpace( int rank, const hsize_t * dims, const hsize_t * maxdims) : IdComponent(0) { id = H5Screate_simple( rank, dims, maxdims ); if( id < 0 ) @@ -103,16 +103,15 @@ DataSpace::DataSpace( const DataSpace& original ) : IdComponent( original ) {} //-------------------------------------------------------------------------- void DataSpace::copy( const DataSpace& like_space ) { - // If this object has a valid id, appropriately decrement reference - // counter and close the id. + // If this object has an hdf5 valid id, close it if( id != H5S_ALL ) { try { - decRefCount(); + close(); } catch (Exception close_error) { throw DataSpaceIException("DataSpace::copy", close_error.getDetailMsg()); } - } // if + } // end if // call C routine to copy the dataspace id = H5Scopy( like_space.getId() ); @@ -134,8 +133,11 @@ void DataSpace::copy( const DataSpace& like_space ) //-------------------------------------------------------------------------- DataSpace& DataSpace::operator=( const DataSpace& rhs ) { - copy(rhs); - return(*this); + if (this != &rhs) + { + copy(rhs); + return(*this); + } } //-------------------------------------------------------------------------- @@ -553,18 +555,17 @@ void DataSpace::selectHyperslab( H5S_seloper_t op, const hsize_t *count, const h //-------------------------------------------------------------------------- void DataSpace::close() { - if( id != H5S_ALL ) // not a constant, should call H5Sclose - { - herr_t ret_value = H5Sclose(id); - if( ret_value < 0 ) - { - throw DataSpaceIException("DataSpace::close", "H5Sclose failed"); - } - // reset the id because the dataspace that it represents is now closed - id = 0; - } - else // cannot close a constant - throw DataSpaceIException("DataSpace::close", "Cannot close a constant"); + // check if id is a valid hdf5 object id before trying to close it + if (p_valid_id(id)) + { + herr_t ret_value = H5Sclose(id); + if( ret_value < 0 ) + { + throw DataSpaceIException("DataSpace::close", "H5Sclose failed"); + } + // reset the id because the dataspace that it represents is now closed + id = 0; + } } //-------------------------------------------------------------------------- @@ -577,16 +578,12 @@ void DataSpace::close() //-------------------------------------------------------------------------- DataSpace::~DataSpace() { - // If this object has a valid id, appropriately decrement reference - // counter and close the id. - if( id != H5S_ALL ) { - try { - decRefCount(); - } - catch (Exception close_error) { - cerr << "DataSpace::~DataSpace - " << close_error.getDetailMsg() << endl; - } - } // if + try { + close(); + } + catch (Exception close_error) { + cerr << "DataSpace::~DataSpace - " << close_error.getDetailMsg() << endl; + } } #ifndef H5_NO_NAMESPACE diff --git a/c++/src/H5DataType.cpp b/c++/src/H5DataType.cpp index 802714e..815faca 100644 --- a/c++/src/H5DataType.cpp +++ b/c++/src/H5DataType.cpp @@ -101,18 +101,17 @@ DataType::DataType(const DataType& original) : H5Object(original) {} //-------------------------------------------------------------------------- void DataType::copy( const DataType& like_type ) { - // reset the identifier of this instance, H5Tclose will be called - // if needed + // If this object is representing an hdf5 object, close it before + // copying like_type to it try { - decRefCount(); + close(); } catch (Exception close_error) { - throw DataTypeIException(inMemFunc("copy"), close_error.getDetailMsg()); + throw FileIException("DataType::copy", close_error.getDetailMsg()); } // call C routine to copy the datatype id = H5Tcopy( like_type.getId() ); - if( id < 0 ) throw DataTypeIException(inMemFunc("copy"), "H5Tcopy failed"); } @@ -130,8 +129,11 @@ void DataType::copy( const DataType& like_type ) //-------------------------------------------------------------------------- DataType& DataType::operator=( const DataType& rhs ) { - copy(rhs); - return(*this); + if (this != &rhs) + { + copy(rhs); + return(*this); + } } //-------------------------------------------------------------------------- @@ -636,13 +638,16 @@ DataSpace DataType::getRegion(void *ref, H5R_type_t ref_type) const //-------------------------------------------------------------------------- void DataType::close() { - herr_t ret_value = H5Tclose(id); - if( ret_value < 0 ) + if (p_valid_id(id)) { - throw DataTypeIException(inMemFunc("close"), "H5Tclose failed"); + herr_t ret_value = H5Tclose(id); + if( ret_value < 0 ) + { + throw DataTypeIException(inMemFunc("close"), "H5Tclose failed"); + } + // reset the id because the datatype that it represents is now closed + id = 0; } - // reset the id because the datatype that it represents is now closed - id = 0; } //-------------------------------------------------------------------------- @@ -655,9 +660,8 @@ void DataType::close() //-------------------------------------------------------------------------- DataType::~DataType() { - // The datatype id will be closed properly try { - decRefCount(); + close(); } catch (Exception close_error) { cerr << inMemFunc("~DataType - ") << close_error.getDetailMsg() << endl; diff --git a/c++/src/H5File.cpp b/c++/src/H5File.cpp index 9550eea..79f2c61 100644 --- a/c++/src/H5File.cpp +++ b/c++/src/H5File.cpp @@ -49,7 +49,7 @@ namespace H5 { ///\brief Default constructor: creates a stub H5File object. // Programmer Binh-Minh Ribler - 2000 //-------------------------------------------------------------------------- -H5File::H5File() : IdComponent() {} +H5File::H5File() : IdComponent(0) {} //-------------------------------------------------------------------------- // Function: H5File overloaded constructor @@ -78,7 +78,7 @@ H5File::H5File() : IdComponent() {} /// http://hdf.ncsa.uiuc.edu/HDF5/doc/RM_H5F.html#File-Create // Programmer Binh-Minh Ribler - 2000 //-------------------------------------------------------------------------- -H5File::H5File( const char* name, unsigned int flags, const FileCreatPropList& create_plist, const FileAccPropList& access_plist ) : IdComponent() +H5File::H5File( const char* name, unsigned int flags, const FileCreatPropList& create_plist, const FileAccPropList& access_plist ) : IdComponent(0) { p_get_file(name, flags, create_plist, access_plist); } @@ -96,7 +96,7 @@ H5File::H5File( const char* name, unsigned int flags, const FileCreatPropList& c /// FileCreatPropList::DEFAULT // Programmer Binh-Minh Ribler - 2000 //-------------------------------------------------------------------------- -H5File::H5File( const H5std_string& name, unsigned int flags, const FileCreatPropList& create_plist, const FileAccPropList& access_plist ) : IdComponent() +H5File::H5File( const H5std_string& name, unsigned int flags, const FileCreatPropList& create_plist, const FileAccPropList& access_plist ) : IdComponent(0) { p_get_file(name.c_str(), flags, create_plist, access_plist); } @@ -258,7 +258,7 @@ void H5File::reOpen() // If this object has a valid id, appropriately decrement reference // counter and close the id. try { - decRefCount(); + close(); } catch (Exception close_error) { throw FileIException("H5File::reOpen", close_error.getDetailMsg()); @@ -649,13 +649,16 @@ hid_t H5File::getLocId() const //-------------------------------------------------------------------------- void H5File::close() { - herr_t ret_value = H5Fclose( id ); - if( ret_value < 0 ) - { - throw FileIException("H5File::close", "H5Fclose failed"); - } - // reset the id because the file that it represents is now closed - id = 0; + if (p_valid_id(id)) + { + herr_t ret_value = H5Fclose( id ); + if( ret_value < 0 ) + { + throw FileIException("H5File::close", "H5Fclose failed"); + } + // reset the id because the file that it represents is now closed + id = 0; + } } //-------------------------------------------------------------------------- @@ -689,12 +692,11 @@ void H5File::throwException(const H5std_string func_name, const H5std_string msg //-------------------------------------------------------------------------- H5File::~H5File() { - // The HDF5 file id will be closed properly try { - decRefCount(); + close(); } catch (Exception close_error) { - cerr << "H5File::~H5File - " << close_error.getDetailMsg() << endl; + cerr << "H5File::~H5File - " << close_error.getDetailMsg() << endl; } } diff --git a/c++/src/H5Group.cpp b/c++/src/H5Group.cpp index 6c0239f..8725b54 100644 --- a/c++/src/H5Group.cpp +++ b/c++/src/H5Group.cpp @@ -187,13 +187,16 @@ DataSpace Group::getRegion(void *ref, H5R_type_t ref_type) const //-------------------------------------------------------------------------- void Group::close() { - herr_t ret_value = H5Gclose( id ); - if( ret_value < 0 ) - { - throw GroupIException("Group::close", "H5Gclose failed"); - } - // reset the id because the group that it represents is now closed - id = 0; + if (p_valid_id(id)) + { + herr_t ret_value = H5Gclose( id ); + if( ret_value < 0 ) + { + throw GroupIException("Group::close", "H5Gclose failed"); + } + // reset the id because the group that it represents is now closed + id = 0; + } } //-------------------------------------------------------------------------- @@ -227,14 +230,12 @@ void Group::throwException(const H5std_string func_name, const H5std_string msg) //-------------------------------------------------------------------------- Group::~Group() { - // The group id will be closed properly try { - decRefCount(); + close(); } catch (Exception close_error) { - cerr << "Group::~Group - " << close_error.getDetailMsg() << endl; + cerr << "Group::~Group - " << close_error.getDetailMsg() << endl; } - } #ifndef H5_NO_NAMESPACE diff --git a/c++/src/H5IdComponent.cpp b/c++/src/H5IdComponent.cpp index d0c60a4..5f56324 100644 --- a/c++/src/H5IdComponent.cpp +++ b/c++/src/H5IdComponent.cpp @@ -43,8 +43,8 @@ IdComponent::IdComponent(const hid_t h5_id) : id(h5_id) {} //-------------------------------------------------------------------------- IdComponent::IdComponent( const IdComponent& original ) { - id = original.id; - incRefCount(); // increment number of references to this id + id = original.id; + incRefCount(); // increment number of references to this id } //-------------------------------------------------------------------------- @@ -80,6 +80,7 @@ void IdComponent::incRefCount() const void IdComponent::decRefCount(const hid_t obj_id) const { if (p_valid_id(obj_id)) + { if (H5Idec_ref(obj_id) < 0) if (H5Iget_ref(obj_id) <= 0) throw IdComponentException(inMemFunc("decRefCount"), @@ -87,6 +88,7 @@ void IdComponent::decRefCount(const hid_t obj_id) const else throw IdComponentException(inMemFunc("decRefCount"), "decrementing object ref count failed"); + } } //-------------------------------------------------------------------------- @@ -112,7 +114,7 @@ int IdComponent::getCounter(const hid_t obj_id) const { counter = H5Iget_ref(obj_id); if (counter < 0) - throw IdComponentException(inMemFunc("incRefCount"), "incrementing object ref count failed"); + throw IdComponentException(inMemFunc("incRefCount"), "getting object ref count failed - negative"); } return (counter); } @@ -168,16 +170,23 @@ H5I_type_t IdComponent::getHDFObjType(const hid_t obj_id) //-------------------------------------------------------------------------- IdComponent& IdComponent::operator=( const IdComponent& rhs ) { - // handling references to this id - decRefCount(); - - // copy the data members from the rhs object - id = rhs.id; - - // increment the reference counter - incRefCount(); - - return( *this ); + if (this != &rhs) + { + // handling references to this id + try { + close(); + } + catch (Exception close_error) { + throw FileIException(inMemFunc("operator="), close_error.getDetailMsg()); + } + + // copy the data members from the rhs object + id = rhs.id; + + // increment the reference counter + incRefCount(); + } + return *this; } //-------------------------------------------------------------------------- @@ -194,11 +203,19 @@ IdComponent& IdComponent::operator=( const IdComponent& rhs ) //-------------------------------------------------------------------------- void IdComponent::setId(const hid_t new_id) { - // handling references to this id - decRefCount(); + // handling references to this old id + try { + close(); + } + catch (Exception close_error) { + throw IdComponentException(inMemFunc("copy"), close_error.getDetailMsg()); + } // reset object's id to the given id id = new_id; + + // increment the reference counter of the new id + incRefCount(); } //-------------------------------------------------------------------------- @@ -209,7 +226,7 @@ void IdComponent::setId(const hid_t new_id) //-------------------------------------------------------------------------- hid_t IdComponent::getId () const { - return( id ); + return(id); } //-------------------------------------------------------------------------- @@ -217,31 +234,7 @@ hid_t IdComponent::getId () const ///\brief Noop destructor. // Programmer Binh-Minh Ribler - 2000 //-------------------------------------------------------------------------- -IdComponent::~IdComponent() { - -/* uncomment this block and complete it when deciding to use dontAtExit - unless the atexit/global destructor problem is fixed, then - remove it- BMR 11/14/00 - - if( id == NOTATEXIT ) - { - // Call H5Library::close to clean up - temporary solution to avoid the - // trouble of atexit/global destructors - try { - if( H5Library::need_cleanup == true ) - { - H5Library::close(); - H5Library::need_cleanup = false; // reset the boolean just in case - } - } - // catch failure caused by the H5Library operations - catch( LibraryIException error ) - { - error.printError(); - } - } -*/ -} +IdComponent::~IdComponent() {} // // Implementation of protected functions for HDF5 Reference Interface. diff --git a/c++/src/H5Object.cpp b/c++/src/H5Object.cpp index 40bed19..84b4efb 100644 --- a/c++/src/H5Object.cpp +++ b/c++/src/H5Object.cpp @@ -53,7 +53,7 @@ extern "C" herr_t userAttrOpWrpr( hid_t loc_id, const char* attr_name, void* op_ // set it to a valid HDF5 id. // Programmer Binh-Minh Ribler - 2000 //-------------------------------------------------------------------------- -H5Object::H5Object() : IdComponent() {} +H5Object::H5Object() : IdComponent(0) {} //-------------------------------------------------------------------------- // Function: H5Object overloaded constructor (protected) diff --git a/c++/src/H5PredType.cpp b/c++/src/H5PredType.cpp index 6f1e5df..510c659 100644 --- a/c++/src/H5PredType.cpp +++ b/c++/src/H5PredType.cpp @@ -237,8 +237,11 @@ const PredType PredType::NATIVE_UINT_LEAST64( H5T_NATIVE_UINT_LEAST64 ); //-------------------------------------------------------------------------- PredType& PredType::operator=( const PredType& rhs ) { - copy(rhs); - return(*this); + if (this != &rhs) + { + copy(rhs); + return(*this); + } } #ifndef DOXYGEN_SHOULD_SKIP_THIS diff --git a/c++/src/H5PropList.cpp b/c++/src/H5PropList.cpp index f4981c3..3457a9d 100644 --- a/c++/src/H5PropList.cpp +++ b/c++/src/H5PropList.cpp @@ -97,20 +97,19 @@ PropList::PropList( const hid_t plist_id ) : IdComponent(0) //-------------------------------------------------------------------------- void PropList::copy( const PropList& like_plist ) { - // If this object has a valid id, appropriately decrement reference - // counter and close the id. + // If this object is representing an hdf5 object, close it before + // copying like_plist to it try { - decRefCount(); + close(); } catch (Exception close_error) { - throw PropListIException(inMemFunc("copy"), close_error.getDetailMsg()); + throw FileIException("PropList::copy", close_error.getDetailMsg()); } - // call C routine to copy the property list - id = H5Pcopy( like_plist.getId() ); - - if( id < 0 ) - throw PropListIException(inMemFunc("copy"), "H5Pcopy failed"); + // call C routine to copy the property list + id = H5Pcopy( like_plist.getId() ); + if( id < 0 ) + throw PropListIException(inMemFunc("copy"), "H5Pcopy failed"); } //-------------------------------------------------------------------------- @@ -126,8 +125,11 @@ void PropList::copy( const PropList& like_plist ) //-------------------------------------------------------------------------- PropList& PropList::operator=( const PropList& rhs ) { - copy(rhs); - return(*this); + if (this != &rhs) + { + copy(rhs); + return(*this); + } } //-------------------------------------------------------------------------- @@ -208,18 +210,16 @@ void PropList::copyProp( PropList& dest, PropList& src, const H5std_string& name //-------------------------------------------------------------------------- void PropList::close() { - if( id != H5P_NO_CLASS ) // not a constant, should call H5Pclose - { - herr_t ret_value = H5Pclose( id ); - if( ret_value < 0 ) - { - throw PropListIException(inMemFunc("close"), "H5Pclose failed"); - } - // reset the id because the property list that it represents is now closed - id = 0; - } - else - throw PropListIException(inMemFunc("close"), "Cannot close a constant"); + if (p_valid_id(id)) + { + herr_t ret_value = H5Pclose( id ); + if( ret_value < 0 ) + { + throw PropListIException(inMemFunc("close"), "H5Pclose failed"); + } + // reset the id because the property list that it represents is now closed + id = 0; + } } //-------------------------------------------------------------------------- @@ -626,9 +626,8 @@ PropList PropList::getClassParent() const //-------------------------------------------------------------------------- PropList::~PropList() { - // The property list id will be closed properly try { - decRefCount(); + close(); } catch (Exception close_error) { cerr << "PropList::~PropList - " << close_error.getDetailMsg() << endl; -- cgit v0.12