diff options
author | Binh-Minh Ribler <bmribler@hdfgroup.org> | 2005-05-22 17:03:56 (GMT) |
---|---|---|
committer | Binh-Minh Ribler <bmribler@hdfgroup.org> | 2005-05-22 17:03:56 (GMT) |
commit | 270c9472a464913e4dc8f884a339561835282ad6 (patch) | |
tree | d067f1f5df469158b6fe2d004006a9f8d9de4891 /c++/src | |
parent | d152a465078e42af8de430dc37cf030108e86458 (diff) | |
download | hdf5-270c9472a464913e4dc8f884a339561835282ad6.zip hdf5-270c9472a464913e4dc8f884a339561835282ad6.tar.gz hdf5-270c9472a464913e4dc8f884a339561835282ad6.tar.bz2 |
[svn-r10781] Purpose: Fix bug (reported by user)
Description:
The use of FileCreatPropList::DEFAULT sometimes caused failure
in the reference counting area. This occurs to all the default
property lists, which also include FileAccPropList::DEFAULT,
DSetCreatPropList::DEFAULT, and DSetMemXferPropList::DEFAULT.
H5P_DEFAULT was used to create these default prop lists and
because its value is 0, the id of these prop lists are 0, which
is rejected by the H5I functions during the reference counting.
Solution:
The main action to fix the above problem was to use
H5P_FILE_CREATE
H5P_FILE_ACCESS
H5P_DATASET_CREATE
H5P_DATASET_XFER
to define the default property lists accordingly. Yet, when this
fix was applied, some bug in reference counting was revealed.
It appeared that some ids were not incremented but were passed in
for decrementing. The following actions were then taken to fix and
improve the current use of reference counting H5I functions.
* added private func IdComponent::p_valid_id to verify that the
id is a valid id and can be passed into an H5I function
* used p_valid_id to validate an id before calling an H5I functions
in the reference-counting member functions incRefCount,
decRefCount, and getCounter
* changed to use member function incRefCount, decRefCount, and
getCounter instead of the C APIs H5Iinc_ref, H5Idec_ref, and
H5Iget_ref throughout IdComponent.
In addition, overloadings were added for incRefCount, decRefCount,
and getCounter to take an id different than the id of the current
instance; they can be convenient during debugging.
H5File.cpp: changed due to the changed xPropList::DEFAULT.
Platforms tested:
Linux 2.4 (heping)
SunOS 5.8 64-bit (sol)
AIX 5.1 (copper)
Diffstat (limited to 'c++/src')
-rw-r--r-- | c++/src/H5DcreatProp.cpp | 2 | ||||
-rw-r--r-- | c++/src/H5DxferProp.cpp | 2 | ||||
-rw-r--r-- | c++/src/H5FaccProp.cpp | 2 | ||||
-rw-r--r-- | c++/src/H5FcreatProp.cpp | 2 | ||||
-rw-r--r-- | c++/src/H5File.cpp | 3 | ||||
-rw-r--r-- | c++/src/H5IdComponent.cpp | 97 | ||||
-rw-r--r-- | c++/src/H5IdComponent.h | 17 | ||||
-rw-r--r-- | c++/src/H5PropList.cpp | 3 |
8 files changed, 103 insertions, 25 deletions
diff --git a/c++/src/H5DcreatProp.cpp b/c++/src/H5DcreatProp.cpp index 231ef72..d960a83 100644 --- a/c++/src/H5DcreatProp.cpp +++ b/c++/src/H5DcreatProp.cpp @@ -30,7 +30,7 @@ namespace H5 { //-------------------------------------------------------------------------- ///\brief Constant for dataset creation default property //-------------------------------------------------------------------------- -const DSetCreatPropList DSetCreatPropList::DEFAULT( H5P_DEFAULT ); +const DSetCreatPropList DSetCreatPropList::DEFAULT; //-------------------------------------------------------------------------- // Function: DSetCreatPropList default constructor diff --git a/c++/src/H5DxferProp.cpp b/c++/src/H5DxferProp.cpp index 8ea596d..46fed7c 100644 --- a/c++/src/H5DxferProp.cpp +++ b/c++/src/H5DxferProp.cpp @@ -27,7 +27,7 @@ namespace H5 { //-------------------------------------------------------------------------- ///\brief Constant for default dataset memory and transfer property list. //-------------------------------------------------------------------------- -const DSetMemXferPropList DSetMemXferPropList::DEFAULT( H5P_DEFAULT ); +const DSetMemXferPropList DSetMemXferPropList::DEFAULT; //-------------------------------------------------------------------------- // Function Default constructor diff --git a/c++/src/H5FaccProp.cpp b/c++/src/H5FaccProp.cpp index b2dd86a..9c27d96 100644 --- a/c++/src/H5FaccProp.cpp +++ b/c++/src/H5FaccProp.cpp @@ -27,7 +27,7 @@ namespace H5 { //-------------------------------------------------------------------------- ///\brief Constant for default property //-------------------------------------------------------------------------- -const FileAccPropList FileAccPropList::DEFAULT( H5P_DEFAULT ); +const FileAccPropList FileAccPropList::DEFAULT; //-------------------------------------------------------------------------- // Function: Default Constructor diff --git a/c++/src/H5FcreatProp.cpp b/c++/src/H5FcreatProp.cpp index a4045fe..1943216 100644 --- a/c++/src/H5FcreatProp.cpp +++ b/c++/src/H5FcreatProp.cpp @@ -27,7 +27,7 @@ namespace H5 { //-------------------------------------------------------------------------- ///\brief Constant for default property //-------------------------------------------------------------------------- -const FileCreatPropList FileCreatPropList::DEFAULT( H5P_DEFAULT ); +const FileCreatPropList FileCreatPropList::DEFAULT; //-------------------------------------------------------------------------- // Function: FileCreatPropList default constructor diff --git a/c++/src/H5File.cpp b/c++/src/H5File.cpp index a376098..d999ee1 100644 --- a/c++/src/H5File.cpp +++ b/c++/src/H5File.cpp @@ -119,8 +119,7 @@ void H5File::p_get_file(const char* name, unsigned int flags, const FileCreatPro // Open the file if none of the bits above are set. else { - // use create_plist for access plist because of the default argument - hid_t access_plist_id = create_plist.getId(); + hid_t access_plist_id = access_plist.getId(); id = H5Fopen( name, flags, access_plist_id ); if( id <= 0 ) // throw an exception when open/create fail { diff --git a/c++/src/H5IdComponent.cpp b/c++/src/H5IdComponent.cpp index e4c5305..7c9457a 100644 --- a/c++/src/H5IdComponent.cpp +++ b/c++/src/H5IdComponent.cpp @@ -13,6 +13,7 @@ * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ #include <string> +#include <iostream> #include "H5Include.h" #include "H5Exception.h" @@ -41,35 +42,89 @@ IdComponent::IdComponent(const hid_t h5_id) : id(h5_id) {} IdComponent::IdComponent( const IdComponent& original ) { id = original.id; - H5Iinc_ref(id); // increment number of references to this id + incRefCount(); // increment number of references to this id } //-------------------------------------------------------------------------- // Function: IdComponent::incRefCount -///\brief Increment id reference counter. +///\brief Increment reference counter for a given id. +// Programmer Binh-Minh Ribler - May 2005 +//-------------------------------------------------------------------------- +void IdComponent::incRefCount(hid_t obj_id) const +{ + if (p_valid_id(obj_id)) + if (H5Iinc_ref(obj_id) < 0) + throw IdComponentException("IdComponent::incRefCount", "incrementing object ref count failed"); +} + +//-------------------------------------------------------------------------- +// Function: IdComponent::incRefCount +///\brief Increment reference counter for the id of this object. // Programmer Binh-Minh Ribler - 2000 //-------------------------------------------------------------------------- -void IdComponent::incRefCount() { H5Iinc_ref(id); } +void IdComponent::incRefCount() const +{ + incRefCount(id); +} + +//-------------------------------------------------------------------------- +// Function: IdComponent::decRefCount +///\brief Decrement reference counter for a given id. +// Programmer Binh-Minh Ribler - May 2005 +// Modification: +// Added the check for ref counter to give a little more info +// on why H5Idec_ref fails in some cases - BMR 5/19/2005 +//-------------------------------------------------------------------------- +void IdComponent::decRefCount(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("IdComponent::decRefCount", + "object ref count is 0 or negative"); + else + throw IdComponentException("IdComponent::decRefCount", + "decrementing object ref count failed"); +} //-------------------------------------------------------------------------- // Function: IdComponent::decRefCount -///\brief Decrement id reference counter. +///\brief Decrement reference counter for the id of this object. // Programmer Binh-Minh Ribler - 2000 //-------------------------------------------------------------------------- -void IdComponent::decRefCount() +void IdComponent::decRefCount() const { - if(id>0) - if(H5Idec_ref(id)<0) - throw IdComponentException("IdComponent::decRefCount", "decrementing object ref count failed"); + decRefCount(id); } //-------------------------------------------------------------------------- // Function: IdComponent::getCounter -///\brief Returns the reference counter to this identifier. +///\brief Returns the reference counter for a given id. +///\return Reference count +// Programmer Binh-Minh Ribler - May 2005 +//-------------------------------------------------------------------------- +int IdComponent::getCounter(hid_t obj_id) const +{ + int counter = 0; + if (p_valid_id(obj_id)) + { + counter = H5Iget_ref(obj_id); + if (counter < 0) + throw IdComponentException("IdComponent::incRefCount", "incrementing object ref count failed"); + } + return (counter); +} + +//-------------------------------------------------------------------------- +// Function: IdComponent::getCounter +///\brief Returns the reference counter for the id of this object. ///\return Reference count // Programmer Binh-Minh Ribler - 2000 //-------------------------------------------------------------------------- -int IdComponent::getCounter() { return( H5Iget_ref(id)); } +int IdComponent::getCounter() const +{ + return (getCounter(id)); +} //-------------------------------------------------------------------------- // Function: IdComponent::operator= @@ -95,7 +150,7 @@ IdComponent& IdComponent::operator=( const IdComponent& rhs ) id = rhs.id; // increment the reference counter - H5Iinc_ref(id); + incRefCount(); return( *this ); } @@ -284,6 +339,26 @@ hid_t IdComponent::p_get_region(void *ref, H5R_type_t ref_type) const return(space_id); } +// +// Local functions used in this class +// + +//-------------------------------------------------------------------------- +// Function: p_valid_id +// Purpose: Verifies that the given id is a valid id so it can be passed +// into an H5I C function. +// Return true if id is valid, false, otherwise +// Programmer Binh-Minh Ribler - May, 2005 +//-------------------------------------------------------------------------- +bool IdComponent::p_valid_id(hid_t obj_id) const +{ + H5I_type_t id_type = H5Iget_type(obj_id); + if (id_type <= H5I_BADID || id_type >= H5I_NGROUPS) + return false; + else + return true; +} + #endif // DOXYGEN_SHOULD_SKIP_THIS #ifndef H5_NO_NAMESPACE diff --git a/c++/src/H5IdComponent.h b/c++/src/H5IdComponent.h index f5cc25e..f775275 100644 --- a/c++/src/H5IdComponent.h +++ b/c++/src/H5IdComponent.h @@ -26,17 +26,16 @@ namespace H5 { class H5_DLLCPP IdComponent { public: // Increment reference counter. - void incRefCount(); + void incRefCount(hid_t obj_id) const; + void incRefCount() const; // Decrement reference counter. - void decRefCount(); + void decRefCount(hid_t obj_id) const; + void decRefCount() const; // Get the reference counter to this identifier. - int getCounter(); - - // Decrements the reference counter then determines if there are no more - // reference to this object - bool noReference(); + int getCounter(hid_t obj_id) const; + int getCounter() const; // Assignment operator IdComponent& operator=( const IdComponent& rhs ); @@ -87,6 +86,10 @@ class H5_DLLCPP IdComponent { // Retrieves a dataspace with the region pointed to selected. hid_t p_get_region(void *ref, H5R_type_t ref_type) const; + + // Verifies that the given id is valid. + bool p_valid_id(hid_t obj_id) const; + #endif // DOXYGEN_SHOULD_SKIP_THIS }; // end class IdComponent diff --git a/c++/src/H5PropList.cpp b/c++/src/H5PropList.cpp index ebf03eb..cb3f211 100644 --- a/c++/src/H5PropList.cpp +++ b/c++/src/H5PropList.cpp @@ -60,6 +60,7 @@ PropList::PropList( const PropList& original ) : IdComponent( original ) {} // property's id to H5P_DEFAULT, otherwise, to the given id. // Note: someone else added this code without comments and this // description was what I came up with from reading the code. +// BMR - 2004 // Programmer Binh-Minh Ribler - 2000 //-------------------------------------------------------------------------- PropList::PropList( const hid_t plist_id ) : IdComponent(0) @@ -591,7 +592,7 @@ PropList::~PropList() { // The property list id will be closed properly try { - decRefCount(); + decRefCount(); } catch (Exception close_error) { cerr << "PropList::~PropList - " << close_error.getDetailMsg() << endl; |