Partitioned communications hang

[ggouaillardet]

This issue was initially reported on Stack Overflow at https://stackoverflow.com/questions/79258925/openmpi5-partitioned-communication-with-multiple-neighbors

With the main branch, the following program running on 3 MPI tasks hang:

#include <mpi.h>
#include <stdio.h>
#include <unistd.h>
#include <string.h>

int main(int argc, char *argv[]) {
  MPI_Init(&argc, &argv);
  int rank, size;
  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
  MPI_Comm_size(MPI_COMM_WORLD, &size);
  if (size != 3) {
    if (rank == 0)
      printf("Need 3 workers, you have %d\n", size);
    return -1;
  }
/*
 *   Topology
 *     0 <-> 1 <-> 2
 *     */
  const int partitions = 1;
  const int count = 16;
  int buffer[partitions * count];
  MPI_Request request[2];

  memset(buffer, 0, sizeof(int) * partitions * count);
  if (rank == 1) {
    for (int i = 0; i < partitions * count; i++) {
      buffer[i] = i;
    }
  }

  int nor = 0;
  if (rank == 0) {
    MPI_Precv_init(buffer, partitions, count, MPI_INT, 1, 0, MPI_COMM_WORLD, MPI_INFO_NULL, &request[nor++]);
  } else if (rank == 1) {
    MPI_Psend_init(buffer, partitions, count, MPI_INT, 0, 0, MPI_COMM_WORLD, MPI_INFO_NULL, &request[nor++]);
    MPI_Psend_init(buffer, partitions, count, MPI_INT, 2, 0, MPI_COMM_WORLD, MPI_INFO_NULL, &request[nor++]);
  } else if (rank == 2) {
    MPI_Precv_init(buffer, partitions, count, MPI_INT, 1, 0, MPI_COMM_WORLD, MPI_INFO_NULL, &request[nor++]);
  }
  MPI_Startall(nor, request);

  if (rank == 1) {
    MPI_Pready(0, request[0]);
    MPI_Pready(0, request[1]);
  }

  MPI_Waitall(nor, request, MPI_STATUS_IGNORE);

  MPI_Barrier(MPI_COMM_WORLD);
  for (int i = 0; i < nor; i++) {
    MPI_Request_free(request + i);
  }
  MPI_Finalize();
  return 0;
}

[ggouaillardet]

The inline patch below fixes the issue, I will review and issue a proper PR sometimes early this week

diff --git a/ompi/mca/part/persist/part_persist.h b/ompi/mca/part/persist/part_persist.h
index eea447c..07043b0 100644
--- a/ompi/mca/part/persist/part_persist.h
+++ b/ompi/mca/part/persist/part_persist.h
@@ -485,9 +485,8 @@ mca_part_persist_start(size_t count, ompi_request_t** requests)
 {
     int err = OMPI_SUCCESS;
     size_t _count = count;
-    size_t i;
 
-    for(i = 0; i < _count && OMPI_SUCCESS == err; i++) {
+    for(size_t i = 0; i < _count && OMPI_SUCCESS == err; i++) {
         mca_part_persist_request_t *req = (mca_part_persist_request_t *)(requests[i]);
         /* First use is a special case, to support lazy initialization */
         if(false == req->first_send)
@@ -503,7 +502,7 @@ mca_part_persist_start(size_t count, ompi_request_t** requests)
         } else {
             if(MCA_PART_PERSIST_REQUEST_PSEND == req->req_type) {
                 req->done_count = 0;
-                for(i = 0; i < req->real_parts && OMPI_SUCCESS == err; i++) {
+                for(size_t i = 0; i < req->real_parts && OMPI_SUCCESS == err; i++) {
                     req->flags[i] = -1;
                 }
             } else {

@mdosanjh I noted there is a lot of code in a header file, and this is not friendly for debuggers.
If there are no objections, I would like to revamp that.

[rhc54]

Hey @ggouaillardet - I can't see the entire code, but does that create a shadow variable? Wouldn't it be wise to rename that second counter to something other than "i"?

[ggouaillardet]

Sure, we can also do that!

[Jonashar]

Thank you for creating the issue and proposing a fix so quickly!

[ggouaillardet]

@Jonashar can you please tell me your full name so I can update the commit message and properly credit you?

[Jonashar]

Oh sorry, of course, my name is Jonas Harlacher.