comm/cid: fix for edge case with intercomm creation

A barrier was removed from the cid allocation algorithm
for intercommunicators that leads to random hangs when
doing MPI_Intercomm_create or moral equivalent.

See issue #12367

the cid allocation algorithm and comm from cid lookup
was modified to use OMPI_COMM_SENTINEL as the placeholder
in the list of pointers to ompi commucators.

Signed-off-by: Howard Pritchard <[email protected]>

Author: hppritcha

ompi/communicator/comm.c

@@ -2200,7 +2200,7 @@ int ompi_comm_free( ompi_communicator_t **comm )
          * makes sure that the pointer to the dependent communicator
          * still contains a valid object.
          */
-        ompi_communicator_t *tmpcomm = (ompi_communicator_t *) opal_pointer_array_get_item(&ompi_mpi_communicators, cid);
+        ompi_communicator_t *tmpcomm = ompi_comm_lookup(cid);
         if ( NULL != tmpcomm ){
             ompi_comm_free(&tmpcomm);
         }

ompi/communicator/comm_cid.c

@@ -590,7 +590,7 @@ static int ompi_comm_allreduce_getnextcid (ompi_comm_request_t *request)
         context->nextlocal_cid = mca_pml.pml_max_contextid;
         for (unsigned int i = context->start ; i < mca_pml.pml_max_contextid ; ++i) {
             flag = opal_pointer_array_test_and_set_item (&ompi_mpi_communicators, i,
-                                                         context->comm);
+                                                         OMPI_COMM_SENTINEL);
             if (true == flag) {
                 context->nextlocal_cid = i;
                 break;
@@ -664,7 +664,7 @@ static int ompi_comm_checkcid (ompi_comm_request_t *request)
             opal_pointer_array_set_item(&ompi_mpi_communicators, context->nextlocal_cid, NULL);
 
             context->flag = opal_pointer_array_test_and_set_item (&ompi_mpi_communicators,
-                                                                  context->nextcid, context->comm);
+                                                                  context->nextcid, OMPI_COMM_SENTINEL);
         }
     }
 
@@ -716,7 +716,7 @@ static int ompi_comm_nextcid_check_flag (ompi_comm_request_t *request)
             for (unsigned int i = context->start ; i < mca_pml.pml_max_contextid ; ++i) {
                 bool flag;
                 flag = opal_pointer_array_test_and_set_item (&ompi_mpi_communicators, i,
-                                                                context->comm);
+                                                             OMPI_COMM_SENTINEL);
                 if (true == flag) {
                     context->nextlocal_cid = i;
                     break;
@@ -1588,4 +1588,3 @@ static int ompi_comm_ft_allreduce_intra_pmix_nb(int *inbuf, int *outbuf, int cou
 }
 
 #endif /* OPAL_ENABLE_FT_MPI */
-

ompi/communicator/comm_init.c

@@ -355,13 +355,13 @@ static int ompi_comm_finalize (void)
     OBJ_DESTRUCT( &ompi_mpi_comm_null );
 
     /* Check whether we have some communicators left */
-    max = opal_pointer_array_get_size(&ompi_mpi_communicators);
+    max = ompi_comm_get_num_communicators();
     for ( i=3; i<max; i++ ) {
-        comm = (ompi_communicator_t *)opal_pointer_array_get_item(&ompi_mpi_communicators, i);
+        comm = ompi_comm_lookup(i);
         if ( NULL != comm ) {
             /* Communicator has not been freed before finalize */
             OBJ_RELEASE(comm);
-            comm=(ompi_communicator_t *)opal_pointer_array_get_item(&ompi_mpi_communicators, i);
+            comm = ompi_comm_lookup(i);
             if ( NULL != comm ) {
                 /* Still here ? */
                 if ( !OMPI_COMM_IS_EXTRA_RETAIN(comm)) {

ompi/communicator/communicator.h

@@ -140,6 +140,11 @@ OMPI_DECLSPEC OBJ_CLASS_DECLARATION(ompi_communicator_t);
 #define OMPI_COMM_BLOCK_WORLD      16
 #define OMPI_COMM_BLOCK_OTHERS     8
 
+/**
+ * Placeholder to use in array of ompi communicators during CID allocation 
+ */
+#define OMPI_COMM_SENTINEL        0x00000001
+
 /* A macro comparing two CIDs */
 #define OMPI_COMM_CID_IS_LOWER(comm1,comm2) ( ((comm1)->c_index < (comm2)->c_index)? 1:0)
 
@@ -552,8 +557,27 @@ static inline bool ompi_comm_compare_cids (const ompi_communicator_t *comm1, con
  * No error checking is done*/
 static inline ompi_communicator_t *ompi_comm_lookup (const uint32_t c_index)
 {
+    ompi_communicator_t *comm = NULL;
     /* array of pointers to communicators, indexed by context ID */
-    return (ompi_communicator_t *) opal_pointer_array_get_item (&ompi_mpi_communicators, c_index);
+    comm = (ompi_communicator_t *) opal_pointer_array_get_item (&ompi_mpi_communicators, c_index);
+    /*
+     * OMPI_COMM_SENTINEL indicates the slot is being used for CID allocation
+     * and is not a valid communicator
+     */
+    if ((ompi_communicator_t *)OMPI_COMM_SENTINEL == comm) {
+        comm = NULL;
+    }
+
+    return comm;
+}
+
+/**
+ * Number of entries in the ompi_mpi_communicators pointer array.
+ * Note this includes entries which may have NULL or OMPI_COMM_SENTINEL values.
+ */
+static inline int ompi_comm_get_num_communicators(void)
+{
+    return opal_pointer_array_get_size(&ompi_mpi_communicators);
 }
 
 static inline ompi_communicator_t *ompi_comm_lookup_cid (const ompi_comm_extended_cid_t cid)

ompi/dpm/dpm.c

@@ -1769,9 +1769,9 @@ int ompi_dpm_dyn_finalize(void)
             return OMPI_ERR_OUT_OF_RESOURCE;
         }
 
-        max = opal_pointer_array_get_size(&ompi_mpi_communicators);
+        max = ompi_comm_get_num_communicators();
         for (i=3; i<max; i++) {
-            comm = (ompi_communicator_t*)opal_pointer_array_get_item(&ompi_mpi_communicators,i);
+            comm = ompi_comm_lookup(i);
             if (NULL != comm &&  OMPI_COMM_IS_DYNAMIC(comm)) {
                 objs[j++] = disconnect_init(comm);
             }

ompi/errhandler/errhandler.c

@@ -355,9 +355,9 @@ int ompi_errhandler_proc_failed_internal(ompi_proc_t* ompi_proc, int status, boo
 
     /* Communicator State:
      * Let them know about the failure. */
-    max_num_comm = opal_pointer_array_get_size(&ompi_mpi_communicators);
+    max_num_comm = ompi_comm_get_num_communicators();
     for( i = 0; i < max_num_comm; ++i ) {
-        comm = (ompi_communicator_t *)opal_pointer_array_get_item(&ompi_mpi_communicators, i);
+        comm = ompi_comm_lookup(i);
         if( NULL == comm ) {
             continue;
         }