[svn-r15395] When an attribute was opened twice and data was written with one of the handles,

the file didn't have the data.  It happened because each handle had its own
object structure, and the empty one overwrote the data with fill value.  This is
fixed by making some attribute information like the data be shared in the
attribute structure.

Tested on smirom, kagiso, and linew.

10 files changed, 56 insertions, 158 deletions

diff --git a/c++/src/H5Attribute.cpp b/c++/src/H5Attribute.cpp
index 87521e1..bdb6276 100644
--- a/c++/src/H5Attribute.cpp
+++ b/c++/src/H5Attribute.cpp
@@ -381,9 +381,6 @@ void Attribute::setId(const hid_t new_id)
     }
    // reset object's id to the given id
    id = new_id;
-
-   // increment the reference counter of the new id
-   incRefCount();
 }
 
 //--------------------------------------------------------------------------
@@ -402,8 +399,10 @@ void Attribute::close()
 	{
 	    throw AttributeIException("Attribute::close", "H5Aclose failed");
 	}
-	// reset the id because the attribute that it represents is now closed
-	id = 0;
+	// reset the id when the attribute that it represents is no longer
+	// referenced
+	if (getCounter() == 0)
+	    id = 0;
     }
 }
 
diff --git a/c++/src/H5DataSet.cpp b/c++/src/H5DataSet.cpp
index be80949..5ee84b1 100644
--- a/c++/src/H5DataSet.cpp
+++ b/c++/src/H5DataSet.cpp
@@ -69,11 +69,10 @@ DataSet::DataSet(const hid_t existing_id) : AbstractDs(), H5Object()
 ///\param	original - IN: DataSet instance to copy
 // Programmer	Binh-Minh Ribler - 2000
 //--------------------------------------------------------------------------
-DataSet::DataSet(const DataSet& original) : AbstractDs(original), H5Object()
+DataSet::DataSet(const DataSet& original) : AbstractDs(original), H5Object(original)
 {
     id = original.getId();
     incRefCount(); // increment number of references to this id
-
 }
 
 //--------------------------------------------------------------------------
@@ -559,7 +558,7 @@ void DataSet::setId(const hid_t new_id)
    id = new_id;
 
    // increment the reference counter of the new id
-   incRefCount();
+   //incRefCount();
 }
 
 //--------------------------------------------------------------------------
@@ -578,8 +577,10 @@ void DataSet::close()
 	{
 	    throw DataSetIException("DataSet::close", "H5Dclose failed");
 	}
-	// reset the id because the dataset that it represents is now closed
-	id = 0;
+	// reset the id when the dataset that it represents is no longer
+	// referenced
+	if (getCounter() == 0)
+	    id = 0;
     }
 }
 
diff --git a/c++/src/H5DataSpace.cpp b/c++/src/H5DataSpace.cpp
index adeb2db..baad137 100644
--- a/c++/src/H5DataSpace.cpp
+++ b/c++/src/H5DataSpace.cpp
@@ -593,9 +593,6 @@ void DataSpace::setId(const hid_t new_id)
     }
    // reset object's id to the given id
    id = new_id;
-
-   // increment the reference counter of the new id
-   incRefCount();
 }
 
 //--------------------------------------------------------------------------
@@ -615,8 +612,10 @@ void DataSpace::close()
 	{
 	    throw DataSpaceIException("DataSpace::close", "H5Sclose failed");
 	}
-	// reset the id because the dataspace that it represents is now closed
-	id = 0;
+	// reset the id when the dataspace that it represents is no longer
+	// referenced
+	if (getCounter() == 0)
+	    id = 0;
     }
 }
 
@@ -632,16 +631,10 @@ void DataSpace::close()
 //--------------------------------------------------------------------------
 DataSpace::~DataSpace()
 {
-    int counter = getCounter(id);
-    if (counter > 1)
-	decRefCount(id);
-    else if (counter == 1)
-    {
-	try {
-	    close();
-	} catch (Exception close_error) {
-	    cerr << "DataSpace::~DataSpace - " << close_error.getDetailMsg() << endl;
-	}
+    try {
+	close();
+    } catch (Exception close_error) {
+	cerr << "DataSpace::~DataSpace - " << close_error.getDetailMsg() << endl;
     }
 }
 
diff --git a/c++/src/H5DataType.cpp b/c++/src/H5DataType.cpp
index 592d800..9575823 100644
--- a/c++/src/H5DataType.cpp
+++ b/c++/src/H5DataType.cpp
@@ -717,9 +717,6 @@ void DataType::setId(const hid_t new_id)
     }
    // reset object's id to the given id
    id = new_id;
-
-   // increment the reference counter of the new id
-   incRefCount();
 }
 
 //--------------------------------------------------------------------------
@@ -738,8 +735,10 @@ void DataType::close()
 	{
 	    throw DataTypeIException(inMemFunc("close"), "H5Tclose failed");
 	}
-	// reset the id because the datatype that it represents is now closed
-	id = 0;
+	// reset the id when the datatype that it represents is no longer
+	// referenced
+	if (getCounter() == 0)
+	    id = 0;
     }
 }
 
@@ -755,19 +754,11 @@ void DataType::close()
 //--------------------------------------------------------------------------
 DataType::~DataType()
 {
-    int counter = getCounter(id);
-    if (counter > 1)
-    {
-	decRefCount(id);
-    }
-    else if (counter == 1)
-    {
 	try {
 	    close();
 	} catch (Exception close_error) {
 	    cerr << inMemFunc("~DataType - ") << close_error.getDetailMsg() << endl;
 	}
-    }
 }
 #ifndef H5_NO_NAMESPACE
 } // end namespace
diff --git a/c++/src/H5File.cpp b/c++/src/H5File.cpp
index 9f8e45f..7cd1936 100644
--- a/c++/src/H5File.cpp
+++ b/c++/src/H5File.cpp
@@ -754,9 +754,6 @@ void H5File::setId(const hid_t new_id)
     }
    // reset object's id to the given id
    id = new_id;
-
-   // increment the reference counter of the new id
-   incRefCount();
 }
 
 //--------------------------------------------------------------------------
@@ -775,8 +772,10 @@ void H5File::close()
 	{
 	    throw FileIException("H5File::close", "H5Fclose failed");
 	}
-	// reset the id because the file that it represents is now closed
-	id = 0;
+	// reset the id when the file that it represents is no longer
+	// referenced
+	if (getCounter() == 0)
+	    id = 0;
     }
 }
 
@@ -813,18 +812,10 @@ void H5File::throwException(const H5std_string& func_name, const H5std_string& m
 //--------------------------------------------------------------------------
 H5File::~H5File()
 {
-    int counter = getCounter(id);
-    if (counter > 1)
-    {
-	decRefCount(id);
-    }
-    else if (counter == 1)
-    {
-	try {
-	    close();
-	} catch (Exception close_error) {
-	    cerr << "H5File::~H5File - " << close_error.getDetailMsg() << endl;
-	}
+    try {
+	close();
+    } catch (Exception close_error) {
+	cerr << "H5File::~H5File - " << close_error.getDetailMsg() << endl;
     }
 }
 
diff --git a/c++/src/H5Group.cpp b/c++/src/H5Group.cpp
index 7ab3b61..6014466 100644
--- a/c++/src/H5Group.cpp
+++ b/c++/src/H5Group.cpp
@@ -192,9 +192,6 @@ void Group::setId(const hid_t new_id)
     }
    // reset object's id to the given id
    id = new_id;
-
-   // increment the reference counter of the new id
-   incRefCount();
 }
 
 //--------------------------------------------------------------------------
@@ -206,8 +203,6 @@ void Group::setId(const hid_t new_id)
 //--------------------------------------------------------------------------
 void Group::close()
 {
- /* cerr << "Group::close/p_valid_id" << endl;
- */ 
     if (p_valid_id(id))
     {
 	herr_t ret_value = H5Gclose( id );
@@ -215,8 +210,10 @@ void Group::close()
 	{
 	    throw GroupIException("Group::close", "H5Gclose failed");
 	}
-	// reset the id because the group that it represents is now closed
-	id = 0;
+	// reset the id when the group that it represents is no longer
+	// referenced
+	if (getCounter() == 0)
+	    id = 0;
     }
 }
 
diff --git a/c++/src/H5IdComponent.cpp b/c++/src/H5IdComponent.cpp
index fa2f3db..b22d869 100644
--- a/c++/src/H5IdComponent.cpp
+++ b/c++/src/H5IdComponent.cpp
@@ -177,59 +177,13 @@ IdComponent& IdComponent::operator=( const IdComponent& rhs )
 
 	// copy the data members from the rhs object
 	setId(rhs.getId());
-	 /* id = rhs.id;
- */ 
+	incRefCount(getId()); // a = b, so there are two objects with the same
+			      // hdf5 id
     }
     return *this;
 }
 
 //--------------------------------------------------------------------------
-// Function:	IdComponent::setId
-///\brief	Sets the identifier of this object to a new value.
-///
-///\exception	H5::IdComponentException when the attempt to close the HDF5
-///		object fails
-// Description:
-// 		The underlaying reference counting in the C library ensures
-// 		that the current valid id of this object is properly closed.
-// 		Then the object's id is reset to the new id.
-// Programmer	Binh-Minh Ribler - 2000
-//--------------------------------------------------------------------------
-#if 0
-void IdComponent::setId(const hid_t new_id)
-{
-    // 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;
- */ 
-   setId(new_id);
-
-   // increment the reference counter of the new id
-   incRefCount();
-}
-#endif
-
-//--------------------------------------------------------------------------
-// Function:	IdComponent::getId
-///\brief	Returns the id of this object
-///\return	HDF5 id
-// Programmer	Binh-Minh Ribler - 2000
-//--------------------------------------------------------------------------
-/*
-hid_t IdComponent::getId () const
-{
-   return(id);
-}
-*/
-
-//--------------------------------------------------------------------------
 // Function:	IdComponent destructor
 ///\brief	Noop destructor.
 // Programmer	Binh-Minh Ribler - 2000
@@ -273,10 +227,7 @@ H5std_string IdComponent::inMemFunc(const char* func_name) const
 ///\brief	Default constructor.
 // Programmer	Binh-Minh Ribler - 2000
 //--------------------------------------------------------------------------
-IdComponent::IdComponent() {
-    /* setId(-1);
- */ 
-}
+IdComponent::IdComponent() {}
 
 //--------------------------------------------------------------------------
 // Function:	IdComponent::p_get_file_name (protected)
@@ -335,31 +286,6 @@ hid_t IdComponent::p_dereference(void* ref)
    return(temp_id);
 }
 
-//--------------------------------------------------------------------------
-// Function:	IdComponent::p_reference (protected)
-// Purpose	Creates a reference to an HDF5 object or a dataset region.
-// Parameters
-//		name - IN: Name of the object to be referenced
-//		dataspace - IN: Dataspace with selection
-//		ref_type - IN: Type of reference; default to \c H5R_DATASET_REGION
-// Return	A reference
-// Exception	H5::IdComponentException
-// Notes	This function is incorrect, and will be removed in the near
-//		future after notifying users of the new APIs ::reference's.
-//		BMR - Oct 8, 2006
-// Programmer	Binh-Minh Ribler - May, 2004
-//--------------------------------------------------------------------------
- /* void* IdComponent::p_reference(const char* name, hid_t space_id, H5R_type_t ref_type) const
-{
-   hobj_ref_t ref;
-   herr_t ret_value = H5Rcreate(&ref, getId(), name, ref_type, space_id);
-   if (ret_value < 0)
-   {
-      throw IdComponentException("", "H5Rcreate failed");
-   }
-   return (reinterpret_cast<void*>(ref));
-}
- */ 
 //
 // Local functions used in this class
 //
diff --git a/c++/src/H5PropList.cpp b/c++/src/H5PropList.cpp
index a65c300..085dfd1 100644
--- a/c++/src/H5PropList.cpp
+++ b/c++/src/H5PropList.cpp
@@ -245,9 +245,6 @@ void PropList::setId(const hid_t new_id)
     }
    // reset object's id to the given id
    id = new_id;
-
-   // increment the reference counter of the new id
-   incRefCount();
 }
 
 //--------------------------------------------------------------------------
@@ -266,8 +263,10 @@ void PropList::close()
 	{
 	    throw PropListIException(inMemFunc("close"), "H5Pclose failed");
 	}
-	// reset the id because the property list that it represents is now closed
-	id = 0;
+	// reset the id when the property list that it represents is no longer
+	// referenced
+	if (getCounter() == 0)
+	    id = 0;
     }
 }
 
diff --git a/c++/test/tattr.cpp b/c++/test/tattr.cpp
index 0d545d7..fe95fe7 100644
--- a/c++/test/tattr.cpp
+++ b/c++/test/tattr.cpp
@@ -1050,6 +1050,11 @@ static void test_attr_dtype_shared()
 	// Open the file again
 	fid1.openFile(FILENAME, H5F_ACC_RDWR);
 
+	// Enclosing to work around the issue of unused variables and/or
+	// objects created by copy constructors stay around until end of 
+	// scope, causing incorrect number of ref counts.
+	{ // First enclosed block
+
 	// Create a datatype to commit and use
 	IntType dtype(PredType::NATIVE_INT);
 
@@ -1065,13 +1070,6 @@ static void test_attr_dtype_shared()
 	// Create dataspace for dataset
 	DataSpace dspace;
 
-	// Enclose the following so that all temporary objects can be
-	// destroyed before testing reference count - this is to overcome
-	// the different time when the temporary objects are to be destroyed
-	// by different compilers.
-	{
-
-	// Create dataset
 	DataSet dset = fid1.createDataSet(DSET1_NAME, dtype, dspace);
 
 #ifndef H5_NO_DEPRECATED_SYMBOLS
@@ -1089,8 +1087,8 @@ static void test_attr_dtype_shared()
 	verify_val((int)statbuf.nlink, 3, "DataSet::getObjinfo", __LINE__, __FILE__);
 #endif /* H5_NO_DEPRECATED_SYMBOLS */
 
-	// Close attribute
-	attr.close();
+        // Close attribute
+        attr.close();
 
 	// Delete attribute
 	dset.removeAttr(ATTR1_NAME);
@@ -1101,8 +1099,8 @@ static void test_attr_dtype_shared()
 	verify_val((int)statbuf.nlink, 2, "DataSet::getObjinfo after DataSet::removeAttr", __LINE__, __FILE__);
 #endif /* H5_NO_DEPRECATED_SYMBOLS */
 
-	// Create attribute on dataset
-	attr = dset.createAttribute(ATTR1_NAME,dtype,dspace);
+        // Create attribute on dataset
+        attr = dset.createAttribute(ATTR1_NAME,dtype,dspace);
 
 #ifndef H5_NO_DEPRECATED_SYMBOLS
 	// Check reference count on named datatype
@@ -1118,11 +1116,15 @@ static void test_attr_dtype_shared()
 	dset.close();
 	dspace.close();
 	dtype.close();
+	} // end of first enclosing
+
 	fid1.close();
 
 	// Open the file again
 	fid1.openFile(FILENAME, H5F_ACC_RDWR);
 
+	{ // Second enclosed block...
+
 	// Open dataset
 	DataSet *dset2 = new DataSet (fid1.openDataSet(DSET1_NAME));
 
@@ -1142,12 +1144,11 @@ static void test_attr_dtype_shared()
 	fid1.getObjinfo(TYPE1_NAME, statbuf);
 	verify_val((int)statbuf.nlink, 3, "DataSet::openAttribute", __LINE__, __FILE__);
 #endif /* H5_NO_DEPRECATED_SYMBOLS */
+	} // end of second enclosing
 
 	// Unlink the dataset
 	fid1.unlink(DSET1_NAME);
 
-        } // end of enclosing to test reference counts
-
 #ifndef H5_NO_DEPRECATED_SYMBOLS
 	// Check reference count on named datatype
 	fid1.getObjinfo(TYPE1_NAME, statbuf);
diff --git a/c++/test/tvlstr.cpp b/c++/test/tvlstr.cpp
index 007ab10..d4d0fd1 100644
--- a/c++/test/tvlstr.cpp
+++ b/c++/test/tvlstr.cpp
@@ -416,7 +416,7 @@ static void test_vlstring_type()
 	file1 = new H5File(FILENAME, H5F_ACC_RDWR);
 
 	// Open the variable-length string datatype just created
-	vlstr_type.setId((file1->openStrType(VLSTR_TYPE)).getId());
+	vlstr_type = file1->openStrType(VLSTR_TYPE);
 
 	// Verify character set and padding
 	cset = vlstr_type.getCset();