From e9162d8fa7b29f694fa5359b2ce31a3943d52ab8 Mon Sep 17 00:00:00 2001
From: Jeff Squyres <jsquyres@cisco.com>
Date: Wed, 20 Oct 2004 22:54:34 +0000
Subject: [PATCH] Back out a recent commit and put the release of attributes
 back in ompi_comm_free() (I recently moved them to the destructor).  They are
 actually more correct in ompi_comm_free() -- there's a lengthy comment in
 comm.c containing the reasons why.

This commit was SVN r3248.
---
 src/communicator/comm.c      | 30 ++++++++++++++++++++++++++++++
 src/communicator/comm_init.c | 11 ++++-------
 2 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/src/communicator/comm.c b/src/communicator/comm.c
index c1e0ad7b44..297773fe02 100644
--- a/src/communicator/comm.c
+++ b/src/communicator/comm.c
@@ -678,7 +678,37 @@ static int ompi_comm_allgather_emulate_intra( void *inbuf, int incount,
 */
 int ompi_comm_free ( ompi_communicator_t **comm )
 {
+    int ret;
+
+    /* Release attributes.  We do this now instead of during the
+       communicator destructor for 2 reasons:
+
+       1. The destructor will only NOT be called immediately during
+          ompi_comm_free() if the reference count is still greater
+          than zero at that point, meaning that there are ongoing
+          communications.  However, pending communications will never
+          need attributes, so it's safe to release them directly here.
+
+       2. Releasing attributes in ompi_comm_free() enables us to check
+          the return status of the attribute delete functions.  At
+          least one interpretation of the MPI standard (i.e., the one
+          of the Intel test suite) is that if any of the attribute
+          deletion functions fail, then MPI_COMM_FREE /
+          MPI_COMM_DISCONNECT should also fail.  We can't do that if
+          we delay releasing the attributes -- we need to release the
+          attributes right away so that we can report the error right
+          away. */
+
+    if (NULL != (*comm)->c_keyhash) {
+        ret = ompi_attr_delete_all(COMM_ATTR, *comm, (*comm)->c_keyhash);
+        if (OMPI_SUCCESS != ret) {
+            return ret;
+        }
+        OBJ_RELEASE((*comm)->c_keyhash);
+    }
+
     /* Release the communicator */
+
     OBJ_RELEASE ( (*comm) );
 
     *comm = MPI_COMM_NULL;
diff --git a/src/communicator/comm_init.c b/src/communicator/comm_init.c
index 2cb58a0520..7980003f38 100644
--- a/src/communicator/comm_init.c
+++ b/src/communicator/comm_init.c
@@ -277,18 +277,15 @@ static void ompi_comm_construct(ompi_communicator_t* comm)
 
 static void ompi_comm_destruct(ompi_communicator_t* comm)
 {
-    /* Release attributes */
-
-    if (NULL != comm->c_keyhash) {
-        ompi_attr_delete_all(COMM_ATTR, comm, comm->c_keyhash);
-        OBJ_RELEASE(comm->c_keyhash);
-    }
+    /* Note that the attributes were already released on this
+       communicator in ompi_comm_free() (i.e., from MPI_COMM_FREE /
+       MPI_COMM_DISCONNECT).  See the lengthy comment in
+       communicator/comm.c in ompi_comm_free() for the reasons why. */
 
     /* Release the collective module */
 
     mca_coll_base_comm_unselect(comm);
 
-
     /*  Check if the communicator is a topology */
     if (OMPI_COMM_IS_CART(comm) || OMPI_COMM_IS_GRAPH(comm)) {